From bc1f2e852be665fd6d14b7cc3213b2fbf5fa3cc4 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sat, 15 Aug 2026 05:36:15 -0500 Subject: [PATCH 01/16] Engines(feat): Add the command execution seam why: Every tmux command forks the binary inline, so an alternative transport -- control mode, a recording, an in-memory fake -- cannot be substituted without copying the library, which is what the downstream work had to do. Connection flags were built in three places that disagreed, so config_file= and colors= reached tmux on some paths and not others. what: - Route dispatch through a TmuxEngine protocol, defaulting to a subprocess engine that forks exactly as before - Accept engine= on Server, and let an engine that names no server of its own adopt the server's connection rather than the ambient one - Derive one ServerConnection for cmd(), raise_if_dead() and fetch_objs() - Read the result's process field defensively, so an engine may return any structurally compatible result rather than only ours - Mark intentional command boundaries with CommandSeparator, so a ";" passed as data can never become one - Report a connection's tmux version behind SupportsTmuxVersion, for callers that render version-gated argv - Keep cmd() returning tmux_cmd, and arguments reaching tmux unchanged, so the default path behaves as it did --- CHANGES | 80 +++++++ docs/api/index.md | 7 + docs/api/libtmux.engines.md | 57 +++++ docs/topics/engines.md | 241 ++++++++++++++++++++ docs/topics/index.md | 7 + src/libtmux/common.py | 147 +++++++----- src/libtmux/engines/__init__.py | 65 ++++++ src/libtmux/engines/base.py | 362 ++++++++++++++++++++++++++++++ src/libtmux/engines/connection.py | 331 +++++++++++++++++++++++++++ src/libtmux/engines/subprocess.py | 312 +++++++++++++++++++++++++ src/libtmux/neo.py | 17 +- src/libtmux/server.py | 180 ++++++++++++--- tests/test_engines.py | 289 ++++++++++++++++++++++++ 13 files changed, 1995 insertions(+), 100 deletions(-) create mode 100644 docs/api/libtmux.engines.md create mode 100644 docs/topics/engines.md create mode 100644 src/libtmux/engines/__init__.py create mode 100644 src/libtmux/engines/base.py create mode 100644 src/libtmux/engines/connection.py create mode 100644 src/libtmux/engines/subprocess.py create mode 100644 tests/test_engines.py diff --git a/CHANGES b/CHANGES index 7a691b0cc9..b8b6d18522 100644 --- a/CHANGES +++ b/CHANGES @@ -45,8 +45,88 @@ $ uvx --from 'libtmux' --prerelease allow python _Notes on the upcoming release will go here._ +### Breaking changes + +#### `raise_if_dead()` no longer echoes tmux's error (#739) + +{meth}`Server.raise_if_dead() ` previously let +tmux write its message straight to the terminal. It now captures that text onto +the raised {exc}`subprocess.CalledProcessError`. The exception type is +unchanged. + +#### `tmux_cmd.process` is a property (#739) + +{attr}`~libtmux.common.tmux_cmd.process` was a plain attribute holding the +{class}`subprocess.Popen` that ran the command; it is now a read-only property. +Reading it is unchanged under the default engine. Assigning to it no longer +works, and reading it after a command ran through an engine that forks no +process raises {exc}`~libtmux.exc.LibTmuxException` rather than returning +`None`. + +### What's new + +#### Pluggable command engines (#739) + +Every tmux command libtmux runs now goes through an *engine* — an object that +takes a rendered argv and returns a structured result. The default, +{class}`~libtmux.engines.subprocess.SubprocessEngine`, forks the tmux binary +exactly as before, so existing code is unaffected. + +Pass `engine=` to {class}`~libtmux.Server` and every command on that server runs +through your object instead. {class}`~libtmux.engines.base.TmuxEngine` is a +{class}`typing.Protocol`, so any object with `run()` and `run_batch()` qualifies +— there is no base class to inherit. That makes it possible to drive libtmux +against a recorded or in-memory tmux with no server running, and it is the seam +the control-mode, asyncio, and native-protocol engines plug into. + +This ships the seam only. {meth}`Server.cmd() ` still +returns a {class}`~libtmux.common.tmux_cmd`, arguments still reach tmux +unchanged, and nothing about the default path is new — an engine is the one +thing you can now replace. + +An engine that names no tmux server of its own adopts the server's connection, +so injecting one into a socket-scoped {class}`~libtmux.Server` cannot silently +dispatch to the ambient tmux server. Engines that name a server keep it. + +{class}`~libtmux.engines.connection.ServerConnection` is now the single place +the tmux binary and the `-L`/`-S`/`-f`/`-2`/`-8` flags are computed; three +separate copies previously disagreed about which flags to emit. It is derived +from the server's public attributes on each use, so reassigning `socket_name` +takes effect on the next command, and it memoizes its {func}`shutil.which` +lookup instead of re-walking `$PATH` for every command. It can also report the +tmux version it targets via +{meth}`~libtmux.engines.connection.ServerConnection.tmux_version`, memoizing one +`tmux -V` probe; an engine that forwards it satisfies the optional +{class}`~libtmux.engines.base.SupportsTmuxVersion` capability, which callers +rendering version-gated argv read to decide whether a flag is safe to send. + +An engine that folds several commands into one dispatch needs to know which `;` +in an argv is a boundary and which is data. +{class}`~libtmux.engines.base.CommandSeparator` marks the boundary and +{func}`~libtmux.engines.base.is_command_separator` finds it, so a `;` a caller +passes as an ordinary argument can never become one by accident. + +See {ref}`engines` for the guide and {ref}`engines-api` for the reference. + +### Fixes + +#### Listing queries honor `config_file` and `colors` (#739) + +{meth}`Server.raise_if_dead() ` and the listing +queries behind {attr}`~libtmux.Server.sessions` built their own connection flags +and emitted only `-L`/`-S`, so a server constructed with `config_file=` or +`colors=` passed those flags on some commands and not others. All paths now +share one connection. A `colors=` value other than `256` or `88` raises +{exc}`~libtmux.exc.UnknownColorOption` on those paths as well. + ### Documentation +#### Engines guide and API reference (#739) + +{ref}`engines` covers what an engine is, writing one, the optional capability +protocols, and explicit command separators. {ref}`engines-api` documents the +module. + #### Cleaner `from_env` examples (#719) The rendered examples for {meth}`Pane.from_env() ` and diff --git a/docs/api/index.md b/docs/api/index.md index 23cd9043b1..8507cab75d 100644 --- a/docs/api/index.md +++ b/docs/api/index.md @@ -96,6 +96,12 @@ Base classes and command execution. Dataclass-based query interface. ::: +:::{grid-item-card} Engine +:link: libtmux.engines +:link-type: doc +How tmux commands are executed, and how to swap that out. +::: + :::{grid-item-card} Options :link: libtmux.options :link-type: doc @@ -173,6 +179,7 @@ Window Pane Client Common +Engine Neo Options Hooks diff --git a/docs/api/libtmux.engines.md b/docs/api/libtmux.engines.md new file mode 100644 index 0000000000..f555001585 --- /dev/null +++ b/docs/api/libtmux.engines.md @@ -0,0 +1,57 @@ +(engines-api)= + +# Engines + +An *engine* is the object that actually runs a tmux command. Every dispatch in +libtmux — {meth}`Server.cmd() `, the listing queries behind +{attr}`~libtmux.Server.sessions`, and {meth}`Server.raise_if_dead() +` — goes through one, and by default that is +{class}`~libtmux.engines.subprocess.SubprocessEngine`, which forks the tmux +binary exactly as libtmux always has. + +The engine is swappable. Pass `engine=` to {class}`~libtmux.Server` and every +command on that server runs through your object instead, which is how you drive +libtmux against a recorded or in-memory tmux without a running server. + +See {ref}`engines` for the guide, with worked examples. + +Every symbol below is re-exported from `libtmux.engines`, so +`from libtmux.engines import SubprocessEngine` works regardless of which +submodule defines it. + +## Requests and results + +A {class}`~libtmux.engines.base.CommandRequest` is a rendered tmux argv; a +{class}`~libtmux.engines.base.CommandResult` is the structured outcome. A +tmux-side failure is *data* here — it sets `returncode` and `stderr` rather than +raising. Only an engine-broken condition (missing binary, lost connection) +raises. + +{class}`~libtmux.engines.base.TmuxEngine` is a {class}`typing.Protocol`, so any +object with `run()` and `run_batch()` is an engine; there is no base class to +inherit. The `Supports*` protocols are optional capabilities an engine may +also implement. + +```{eval-rst} +.. automodule:: libtmux.engines.base + :members: +``` + +## Connections + +A {class}`~libtmux.engines.connection.ServerConnection` is the pair every engine +needs before it can dispatch anything: which tmux *binary* to run, and the +connection flags (`-L`/`-S`/`-f`/`-2`/`-8`) naming one tmux server. It is the +single place either is computed. + +```{eval-rst} +.. automodule:: libtmux.engines.connection + :members: +``` + +## The default engine + +```{eval-rst} +.. automodule:: libtmux.engines.subprocess + :members: +``` diff --git a/docs/topics/engines.md b/docs/topics/engines.md new file mode 100644 index 0000000000..29aebc121c --- /dev/null +++ b/docs/topics/engines.md @@ -0,0 +1,241 @@ +(engines)= + +# Engines + +Every tmux command libtmux runs goes through an **engine**. An engine takes a +rendered argv and returns a structured result — that is its whole job. + +By default that engine is +{class}`~libtmux.engines.subprocess.SubprocessEngine`, which forks the tmux +binary once per command. You never have to know it exists. But because it is a +seam rather than hard-wired code, you can replace it — to test without tmux +running, to record what libtmux would do, or to point one `Server` at a +different tmux binary than another. + +## The default path + +Nothing changes if you ignore engines entirely: + +```python +>>> server.cmd("display-message", "-p", "#{session_name}").stdout +['libtmux_...'] +``` + +Under that call, {class}`~libtmux.Server` built a +{class}`~libtmux.engines.connection.ServerConnection` from its own +`socket_name`, `socket_path`, `config_file`, and `colors`, handed it to a +`SubprocessEngine`, and asked the engine to run the command: + +```python +>>> from libtmux.engines import SubprocessEngine +>>> server.connection.args +('-L...',) +>>> isinstance(server.engine, SubprocessEngine) +True +``` + +The connection is *derived*, not frozen at construction, so moving a server to a +different socket is picked up on the next command: + +```python +>>> from libtmux.server import Server +>>> tmux = Server(socket_name="engines_doc_a") +>>> tmux.connection.args +('-Lengines_doc_a',) +>>> tmux.socket_name = "engines_doc_b" +>>> tmux.connection.args +('-Lengines_doc_b',) +``` + +## Requests and results + +An engine speaks two value types. +{class}`~libtmux.engines.base.CommandRequest` is the argv *after* the binary and +connection flags. {class}`~libtmux.engines.base.CommandResult` is what came +back. + +```python +>>> from libtmux.engines import CommandRequest +>>> CommandRequest.from_args("kill-window", "-t", 2) +CommandRequest(args=('kill-window', '-t', '2'), tmux_bin=None) +``` + +A tmux-side failure is **data**, not an exception. An engine sets `returncode` +and `stderr`; it does not raise. Only an engine-broken condition — a missing +binary, a dropped connection — raises: + +```python +>>> from libtmux.engines import CommandResult +>>> result = CommandResult( +... cmd=("tmux", "kill-window"), +... stderr=("no such window",), +... returncode=1, +... ) +>>> result.returncode, result.stderr +(1, ('no such window',)) +``` + +## Writing an engine + +{class}`~libtmux.engines.base.TmuxEngine` is a {class}`typing.Protocol`. There +is no base class to inherit — any object with `run()` and `run_batch()` is an +engine. + +Here is a complete one that runs nothing, records everything, and answers from a +canned script. Hand it to a server and no tmux process is involved: + +```python +>>> from libtmux.engines import CommandResult +>>> from libtmux.server import Server + +>>> class RecordingEngine: +... """Record every dispatch; answer from a canned script.""" +... +... def __init__(self, stdout=()): +... self.requests = [] +... self._stdout = tuple(stdout) +... +... def run(self, request): +... self.requests.append(request.args) +... return CommandResult(cmd=("tmux", *request.args), stdout=self._stdout) +... +... def run_batch(self, requests): +... return [self.run(request) for request in requests] + +>>> recorder = RecordingEngine(stdout=("my_session",)) +>>> offline = Server(engine=recorder) +>>> offline.cmd("display-message", "-p", "#{session_name}").stdout +['my_session'] +>>> recorder.requests +[('display-message', '-p', '#{session_name}')] +``` + +This works because the socket flags live on the *engine*, not in the request, so +your `run()` only ever sees the tmux subcommand — never a `-L` to parse back +out: + +```python +>>> from libtmux.engines import CommandResult +>>> from libtmux.server import Server + +>>> class Recorder: +... def __init__(self): +... self.requests = [] +... def run(self, request): +... self.requests.append(request.args) +... return CommandResult(cmd=("tmux", *request.args)) +... def run_batch(self, requests): +... return [self.run(request) for request in requests] + +>>> recorder = Recorder() +>>> _ = Server(socket_name="engines_doc_scoped", engine=recorder).cmd("list-sessions") +>>> recorder.requests +[('list-sessions',)] +``` + +## Injected engines and sockets + +An engine that names no tmux server of its own **adopts** the server's +connection. Without that rule, injecting a bare engine into a socket-scoped +server would silently dispatch to whichever server a flagless `tmux` reaches: + +```python +>>> from libtmux.engines import SubprocessEngine +>>> from libtmux.server import Server +>>> scoped = Server(socket_name="engines_doc_c", engine=SubprocessEngine()) +>>> scoped.engine.server_args +('-Lengines_doc_c',) +``` + +An engine that *does* name a server is left exactly as you built it: + +```python +>>> from libtmux.engines import SubprocessEngine +>>> from libtmux.server import Server +>>> pinned = SubprocessEngine.of(server_args=("-Lengines_doc_pinned",)) +>>> Server(socket_name="engines_doc_c", engine=pinned).engine.server_args +('-Lengines_doc_pinned',) +``` + +An in-memory engine has no connection at all, so neither rule applies and it is +used untouched. + +## Optional capabilities + +An engine may implement extra protocols. Each is optional; libtmux checks with +{func}`isinstance` and degrades gracefully when absent. + +{class}`~libtmux.engines.base.SupportsCommandLine` renders the argv an engine +*would* run, which is how the full command line reaches the debug log before +dispatch. {class}`~libtmux.engines.base.SupportsConnection` marks an engine that +dispatches over a named server and can be rebound — the protocol behind the +adoption rule above. + +```python +>>> from libtmux.engines import ( +... SubprocessEngine, +... SupportsCommandLine, +... SupportsConnection, +... ) +>>> engine = SubprocessEngine() +>>> isinstance(engine, SupportsCommandLine), isinstance(engine, SupportsConnection) +(True, True) +``` + +An engine that implements neither simply is not matched: + +```python +>>> from libtmux.engines import CommandResult, SupportsCommandLine +>>> class Bare: +... def run(self, request): +... return CommandResult(cmd=("tmux", *request.args)) +... def run_batch(self, requests): +... return [self.run(request) for request in requests] +>>> isinstance(Bare(), SupportsCommandLine) +False +``` + +{class}`~libtmux.engines.base.SupportsTmuxVersion` reports the tmux version an +engine targets, which a caller rendering version-gated argv reads to decide +whether a flag is safe to send. An engine that cannot know its version — an +in-memory fake — omits it, and the caller assumes the newest tmux. + +## Explicit command separators + +tmux treats a bare `;` argument as a boundary between two commands, but only +when it arrives unquoted. A `;` that is *data* — a pane title, a shell fragment +bound for `send-keys` — must not be mistaken for one. Guessing from the string +alone cannot tell them apart, so the intent rides in the type: +{class}`~libtmux.engines.base.CommandSeparator` marks a real boundary, and +{func}`~libtmux.engines.base.is_command_separator` finds it. + +```python +>>> from libtmux.engines import CommandRequest, CommandSeparator, is_command_separator +>>> request = CommandRequest.from_args( +... "rename-window", "a;b", CommandSeparator(";"), "kill-window", "@2" +... ) +>>> [is_command_separator(arg) for arg in request.args] +[False, False, True, False, False] +``` + +A plain `";"` is data and stays data, so nothing an existing caller passes can +become a boundary by accident: + +```python +>>> from libtmux.engines import is_command_separator +>>> is_command_separator(";") +False +``` + +The marker survives normalization, so an engine that chains commands into one +dispatch can find the boundaries while every other engine ignores them. The +default {class}`~libtmux.engines.subprocess.SubprocessEngine` sends one command +per dispatch and has no use for them. + +## What an engine does not change + +An engine chooses *how* a command runs, not what libtmux does with the answer. +Arguments reach tmux exactly as they always have, results read exactly as they +always have, and {meth}`Server.cmd() ` still returns a +{class}`~libtmux.common.tmux_cmd`. Under the default engine there is nothing new +to learn and nothing to migrate. diff --git a/docs/topics/index.md b/docs/topics/index.md index c955e5857e..5b4653c3d6 100644 --- a/docs/topics/index.md +++ b/docs/topics/index.md @@ -61,6 +61,12 @@ Common patterns for scripting and automation. Automatic cleanup with temporary sessions and windows. ::: +:::{grid-item-card} Engines +:link: engines +:link-type: doc +Swap how tmux commands execute: record, fake, or retarget the binary. +::: + :::{grid-item-card} Options & Hooks :link: options_and_hooks :link-type: doc @@ -97,6 +103,7 @@ workspace_setup automation_patterns context_managers options_and_hooks +engines clients format-tokens ``` diff --git a/src/libtmux/common.py b/src/libtmux/common.py index 2871547700..f496e84dde 100644 --- a/src/libtmux/common.py +++ b/src/libtmux/common.py @@ -11,17 +11,20 @@ import logging import re import shlex -import shutil -import subprocess import sys import typing as t from . import exc from ._compat import LooseVersion +from .engines.base import CommandRequest, SupportsCommandLine +from .engines.subprocess import SubprocessEngine if t.TYPE_CHECKING: + import subprocess from collections.abc import Callable + from .engines.base import TmuxEngine + logger = logging.getLogger(__name__) @@ -281,7 +284,35 @@ def raise_if_stderr(proc: tmux_cmd, subcommand: str) -> None: class tmux_cmd: - """Run any :term:`tmux(1)` command through :py:mod:`subprocess`. + """Run any :term:`tmux(1)` command, returning list-shaped output. + + Dispatches through a :class:`~libtmux.engines.base.TmuxEngine` -- + :class:`~libtmux.engines.subprocess.SubprocessEngine` unless one is passed -- + and adapts the engine's :class:`~libtmux.engines.base.CommandResult` to the + ``list``-of-``str`` attributes libtmux's wrappers read. + + Parameters + ---------- + *args : typing.Any + tmux argv. Connection flags may be included inline (``"-Lwork"``); an + engine supplies its own, so :meth:`libtmux.Server.cmd` passes only the + subcommand. + tmux_bin : str, optional + Path to the tmux binary. Ignored when *engine* is given -- the engine + owns its binary. + engine : :class:`~libtmux.engines.base.TmuxEngine`, optional + Executor to dispatch through. + + Attributes + ---------- + cmd : list[str] + The full argv that ran, tmux binary first. + stdout : list[str] + Standard output, one line per item. + stderr : list[str] + Standard error, one line per item, blanks removed. + returncode : int + tmux exit code. Examples -------- @@ -309,66 +340,57 @@ class tmux_cmd: Renamed from ``tmux`` to ``tmux_cmd``. """ - def __init__(self, *args: t.Any, tmux_bin: str | None = None) -> None: - resolved = tmux_bin or shutil.which("tmux") - if not resolved: - raise exc.TmuxCommandNotFound - - cmd = [resolved] - cmd += args # add the command arguments to cmd - cmd = [str(c) for c in cmd] - - self.cmd = cmd + def __init__( + self, + *args: t.Any, + tmux_bin: str | None = None, + engine: TmuxEngine | None = None, + ) -> None: + runner: TmuxEngine = ( + engine if engine is not None else SubprocessEngine.of(tmux_bin) + ) + request = CommandRequest.from_args(*args) if logger.isEnabledFor(logging.DEBUG): - cmd_str = shlex.join(cmd) logger.debug( "tmux command dispatched", - extra={"tmux_cmd": cmd_str}, - ) - - try: - self.process = subprocess.Popen( - cmd, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - text=True, - encoding="utf-8", - errors="backslashreplace", - ) - stdout, stderr = self.process.communicate() - returncode = self.process.returncode - except FileNotFoundError: - raise exc.TmuxCommandNotFound from None - except Exception: - logger.error( # noqa: TRY400 - "tmux subprocess failed", extra={ - "tmux_cmd": shlex.join(cmd), + "tmux_cmd": shlex.join( + runner.command_line(request) + if isinstance(runner, SupportsCommandLine) + else request.args, + ), + "tmux_subcommand": request.subcommand, }, ) - raise - - self.returncode = returncode - - stdout_split = stdout.split("\n") - # remove trailing newlines from stdout - while stdout_split and stdout_split[-1] == "": - stdout_split.pop() - stderr_split = stderr.split("\n") - self.stderr = list(filter(None, stderr_split)) # filter empty values - - if "has-session" in cmd and len(self.stderr) and not stdout_split: - self.stdout = [self.stderr[0]] - else: - self.stdout = stdout_split + result = runner.run(request) + + self.cmd = list(result.cmd) + self.returncode = result.returncode + self.stderr = list(result.stderr) + # Read defensively: ``process`` is the one field of ``CommandResult`` + # that no protocol declares, so an engine returning its own + # result type -- which ``TmuxEngine`` permits -- need not carry it. + process: subprocess.Popen[str] | None = getattr(result, "process", None) + self._process = process + + # tmux writes ``has-session``'s answer to stderr; the wrappers have + # always read it off stdout. Adapted here, not in an engine, so every + # engine stays a plain executor. + stdout = list(result.stdout) + self.stdout = ( + [self.stderr[0]] + if "has-session" in self.cmd and self.stderr and not stdout + else stdout + ) if logger.isEnabledFor(logging.DEBUG): logger.debug( "tmux command completed", extra={ - "tmux_cmd": shlex.join(cmd), + "tmux_cmd": shlex.join(self.cmd), + "tmux_subcommand": request.subcommand, "tmux_exit_code": self.returncode, "tmux_stdout": self.stdout[:100], "tmux_stderr": self.stderr[:100], @@ -377,6 +399,31 @@ def __init__(self, *args: t.Any, tmux_bin: str | None = None) -> None: }, ) + @property + def process(self) -> subprocess.Popen[str]: + """Return the finished :class:`subprocess.Popen`. + + Returns + ------- + subprocess.Popen + The process the default engine forked. + + Raises + ------ + :exc:`~libtmux.exc.LibTmuxException` + The engine that ran the command never forked a process. Only an + injected engine can do that; the default engine always forks. + + Examples + -------- + >>> server.cmd("display-message", "-p", "hi").process.returncode + 0 + """ + if self._process is None: + msg = "engine did not fork a subprocess; tmux_cmd.process is unavailable" + raise exc.LibTmuxException(msg) + return self._process + class _TmuxVersionUnavailable(Exception): """Internal signal: this tmux predates the ``-V`` flag (pre-1.7).""" diff --git a/src/libtmux/engines/__init__.py b/src/libtmux/engines/__init__.py new file mode 100644 index 0000000000..01eaabe4c8 --- /dev/null +++ b/src/libtmux/engines/__init__.py @@ -0,0 +1,65 @@ +"""Engines: the seam between libtmux's object API and tmux itself. + +An *engine* answers one question -- how does a tmux command actually get run? +:class:`~libtmux.engines.subprocess.SubprocessEngine` is the default and forks the +tmux CLI, which is what libtmux has always done. Because +:class:`~libtmux.engines.base.TmuxEngine` is a +:class:`typing.Protocol`, an in-memory fake, a recorder, or a control-mode client +can take its place: + +>>> from libtmux.engines import CommandRequest, CommandResult, TmuxEngine +>>> class SpyEngine: +... def __init__(self): +... self.seen: list[tuple[str, ...]] = [] +... +... def run(self, request): +... self.seen.append(request.args) +... return CommandResult(cmd=("tmux", *request.args), stdout=("$1",)) +... +... def run_batch(self, requests): +... return [self.run(request) for request in requests] +>>> engine = SpyEngine() +>>> isinstance(engine, TmuxEngine) +True + +Injection happens at the :class:`~libtmux.Server` boundary: + +>>> from libtmux.server import Server +>>> Server(socket_name="engine_docs", engine=engine).cmd("list-sessions").stdout +['$1'] +>>> engine.seen +[('list-sessions',)] + +The connection flags (``-L``/``-S``/``-f``/``-2``/``-8``) are *not* part of a +request: they belong to the engine's +:class:`~libtmux.engines.connection.ServerConnection`, so every engine sees the +same request regardless of which tmux server it targets. +""" + +from __future__ import annotations + +from libtmux.engines.base import ( + CommandRequest, + CommandResult, + CommandSeparator, + SupportsCommandLine, + SupportsConnection, + SupportsTmuxVersion, + TmuxEngine, + is_command_separator, +) +from libtmux.engines.connection import ServerConnection +from libtmux.engines.subprocess import SubprocessEngine + +__all__ = ( + "CommandRequest", + "CommandResult", + "CommandSeparator", + "ServerConnection", + "SubprocessEngine", + "SupportsCommandLine", + "SupportsConnection", + "SupportsTmuxVersion", + "TmuxEngine", + "is_command_separator", +) diff --git a/src/libtmux/engines/base.py b/src/libtmux/engines/base.py new file mode 100644 index 0000000000..36c727c9c7 --- /dev/null +++ b/src/libtmux/engines/base.py @@ -0,0 +1,362 @@ +"""Core engine values: requests, results, and the protocols. + +A :class:`CommandRequest` is a tmux argv (the subcommand and its arguments, +*without* connection flags); a :class:`CommandResult` is the structured outcome. +:class:`TmuxEngine` is a :class:`typing.Protocol`, so any object with ``run`` and +``run_batch`` is an engine -- an in-memory fake, a control-mode client, a +recorder -- without inheriting a base class. +""" + +from __future__ import annotations + +import typing as t +from dataclasses import dataclass, field + +if t.TYPE_CHECKING: + import pathlib + import subprocess + from collections.abc import Sequence + + from typing_extensions import Self + + +class CommandSeparator(str): + """A caller-authored command boundary, distinct from a literal ``";"``. + + tmux treats a bare ``;`` argument as a command separator only when it + arrives unquoted, so a ``";"`` that is *data* -- a pane title, a shell + fragment passed to ``send-keys`` -- must not be mistaken for one. Marking + the boundary with its own type keeps the distinction in the value rather + than in a parsing convention, so an engine that chains commands can find + the real boundaries and every other engine can ignore them. + + Examples + -------- + >>> CommandSeparator(";") + ';' + >>> CommandSeparator("kill-server") + Traceback (most recent call last): + ... + ValueError: a command separator must be exactly ';' + """ + + def __new__(cls, value: str) -> Self: + """Construct the one legal structural token. + + Parameters + ---------- + value : str + Must be exactly ``";"``. + + Returns + ------- + CommandSeparator + The separator. + + Raises + ------ + ValueError + *value* is anything other than ``";"``. + """ + if value != ";": + msg = "a command separator must be exactly ';'" + raise ValueError(msg) + return super().__new__(cls, value) + + +def is_command_separator(token: str) -> bool: + """Return whether *token* is an intentional tmux command boundary. + + A plain ``";"`` is data and answers ``False``; only a + :class:`CommandSeparator` answers ``True``. + + Parameters + ---------- + token : str + The argv token to test. + + Returns + ------- + bool + + Examples + -------- + >>> is_command_separator(CommandSeparator(";")) + True + >>> is_command_separator(";") + False + """ + return type(token) is CommandSeparator and token == ";" + + +@dataclass(frozen=True) +class CommandRequest: + """A tmux command, ready for an engine to execute. + + Carries the subcommand and its arguments only. Connection flags + (``-L``/``-S``/``-f``/``-2``/``-8``) belong to the engine's + :class:`~libtmux.engines.connection.ServerConnection`, so every engine sees + the same request no matter which tmux server it targets. + + Attributes + ---------- + args : tuple[str, ...] + The tmux argv (e.g. ``("split-window", "-t", "%1")``). + tmux_bin : str or None + Override the tmux binary for this one request; ``None`` lets the engine + decide. + + Examples + -------- + >>> CommandRequest.from_args("split-window", "-t", "%1") + CommandRequest(args=('split-window', '-t', '%1'), tmux_bin=None) + >>> CommandRequest.from_args("kill-window", "-t", 2).args + ('kill-window', '-t', '2') + """ + + args: tuple[str, ...] + tmux_bin: str | None = None + + def __post_init__(self) -> None: + r"""Reject arguments that cannot survive tmux's C-string transports. + + Examples + -------- + >>> CommandRequest(args=("display-message", "a\0b")) + Traceback (most recent call last): + ... + ValueError: tmux command arguments cannot contain NUL + + A :class:`CommandSeparator` keeps its type through normalization, so a + chaining engine can still find the boundary: + + >>> request = CommandRequest(args=("kill-window", CommandSeparator(";"))) + >>> [is_command_separator(arg) for arg in request.args] + [False, True] + + A separator whose value was forged past :meth:`CommandSeparator.__new__` + is rejected rather than passed through as structural, so it cannot + smuggle a second command into a chained dispatch: + + >>> CommandRequest.from_args("display-message", str.__new__( + ... CommandSeparator, "\nkill-server")) + Traceback (most recent call last): + ... + ValueError: a command separator must be exactly ';' + """ + if any( + type(arg) is CommandSeparator and not is_command_separator(arg) + for arg in self.args + ): + msg = "a command separator must be exactly ';'" + raise ValueError(msg) + normalized = tuple( + arg if is_command_separator(arg) else str.__str__(arg) for arg in self.args + ) + if any("\0" in arg for arg in normalized): + msg = "tmux command arguments cannot contain NUL" + raise ValueError(msg) + object.__setattr__(self, "args", normalized) + + @classmethod + def from_args( + cls, + *args: t.Any, + tmux_bin: str | pathlib.Path | None = None, + ) -> CommandRequest: + """Build a request from arbitrary tokens, stringifying each. + + Parameters + ---------- + *args : typing.Any + Tokens; non-strings are rendered with :func:`str`, matching what + :class:`~libtmux.common.tmux_cmd` has always accepted. + tmux_bin : str or pathlib.Path, optional + Per-request tmux binary override. + + Returns + ------- + CommandRequest + The request. + + Examples + -------- + >>> CommandRequest.from_args("resize-pane", "-t", "%3", "-x", 80).args + ('resize-pane', '-t', '%3', '-x', '80') + """ + return cls( + args=tuple(arg if isinstance(arg, str) else str(arg) for arg in args), + tmux_bin=str(tmux_bin) if tmux_bin is not None else None, + ) + + @property + def subcommand(self) -> str: + """Return the tmux subcommand, or ``""`` for an empty request. + + Returns + ------- + str + First argv token. + + Examples + -------- + >>> CommandRequest.from_args("list-sessions", "-F#S").subcommand + 'list-sessions' + >>> CommandRequest.from_args().subcommand + '' + """ + return self.args[0] if self.args else "" + + +@dataclass(frozen=True) +class CommandResult: + """The structured outcome of executing a :class:`CommandRequest`. + + A tmux-side failure (nonzero exit, message on stderr) is *data* here: it + sets ``returncode`` and ``stderr`` rather than raising. Only a broken engine + (missing binary, lost connection) raises. + + Attributes + ---------- + cmd : tuple[str, ...] + The full argv that ran, including the tmux binary and connection flags. + stdout : tuple[str, ...] + Captured standard-output lines, trailing blanks removed. + stderr : tuple[str, ...] + Captured standard-error lines, blanks removed. + returncode : int + tmux exit code. + process : subprocess.Popen or None + The OS process, when the engine forked one. ``None`` for engines that + never touch the operating system, which is why + :attr:`libtmux.common.tmux_cmd.process` can only be a best-effort + accessor. Excluded from equality and :func:`repr`. + + Examples + -------- + >>> CommandResult(cmd=("tmux", "display-message", "-p", "hi"), stdout=("hi",)) + CommandResult(cmd=('tmux', 'display-message', '-p', 'hi'), stdout=('hi',), + stderr=(), returncode=0) + """ + + cmd: tuple[str, ...] + stdout: tuple[str, ...] = () + stderr: tuple[str, ...] = () + returncode: int = 0 + process: subprocess.Popen[str] | None = field( + default=None, + compare=False, + repr=False, + ) + + +@t.runtime_checkable +class TmuxEngine(t.Protocol): + """A synchronous executor of tmux commands. + + Structural: an object is an engine when it has ``run`` and ``run_batch``. + + Examples + -------- + >>> from libtmux.engines import CommandRequest, CommandResult, TmuxEngine + >>> class EchoEngine: + ... def run(self, request): + ... return CommandResult(cmd=("tmux", *request.args), stdout=("ok",)) + ... + ... def run_batch(self, requests): + ... return [self.run(request) for request in requests] + >>> isinstance(EchoEngine(), TmuxEngine) + True + >>> EchoEngine().run(CommandRequest.from_args("list-sessions")).stdout + ('ok',) + """ + + def run(self, request: CommandRequest) -> CommandResult: + """Execute one tmux command and return its structured result.""" + ... + + def run_batch(self, requests: Sequence[CommandRequest]) -> list[CommandResult]: + """Execute requests in order, returning one result per request. + + Persistent-connection engines override this to pipeline; stateless + engines implement it as a loop over :meth:`run`. + """ + ... + + +@t.runtime_checkable +class SupportsCommandLine(t.Protocol): + """An engine that can render the argv it *would* run, without running it. + + Optional capability. :class:`~libtmux.common.tmux_cmd` uses it to log the + full command line before dispatch; engines without a command line (in-memory + fakes) simply do not implement it. + + Examples + -------- + >>> from libtmux.engines import SupportsCommandLine, SubprocessEngine + >>> isinstance(SubprocessEngine.for_server(server), SupportsCommandLine) + True + """ + + def command_line(self, request: CommandRequest) -> tuple[str, ...]: + """Return the full argv, binary first, that *request* would run as.""" + ... + + +@t.runtime_checkable +class SupportsConnection(t.Protocol): + """An engine that dispatches over a named tmux server and can be rebound. + + Optional capability. :attr:`Server.engine ` reads it + so an injected engine that names no server of its own adopts the server's + connection instead of silently reaching the ambient tmux server. In-memory + engines have no connection and simply do not implement it. + + Examples + -------- + >>> from libtmux.engines import SubprocessEngine, SupportsConnection + >>> isinstance(SubprocessEngine(), SupportsConnection) + True + + An engine with no notion of a socket does not implement it: + + >>> class InMemoryEngine: + ... def run(self, request): + ... return CommandResult(cmd=("tmux", *request.args)) + ... def run_batch(self, requests): + ... return [self.run(r) for r in requests] + >>> isinstance(InMemoryEngine(), SupportsConnection) + False + """ + + @property + def connection(self) -> t.Any: + """Return the tmux binary and flags this engine dispatches over.""" + ... + + def with_connection(self, connection: t.Any) -> TmuxEngine: + """Return an equivalent engine bound to *connection*.""" + ... + + +@t.runtime_checkable +class SupportsTmuxVersion(t.Protocol): + """An engine that can report the tmux version it targets. + + Optional capability. Callers that render version-gated argv -- dropping a + flag an older tmux cannot accept -- read it to resolve the version when + none is passed. Engines that cannot know their version, such as in-memory + fakes, simply do not implement it, and resolution falls back to assuming + the newest tmux. + + Examples + -------- + >>> from libtmux.engines import SubprocessEngine, SupportsTmuxVersion + >>> isinstance(SubprocessEngine.for_server(server), SupportsTmuxVersion) + True + """ + + def tmux_version(self) -> str | None: + """Return the engine's tmux version string, or ``None`` if unknown.""" + ... diff --git a/src/libtmux/engines/connection.py b/src/libtmux/engines/connection.py new file mode 100644 index 0000000000..d2043838ec --- /dev/null +++ b/src/libtmux/engines/connection.py @@ -0,0 +1,331 @@ +"""The connection an engine talks to: which tmux binary, which tmux server. + +Every engine needs the same two things before it can dispatch anything: a tmux +*binary* to exec, and the *connection flags* (``-L``/``-S``/``-f``/``-2``/``-8``) +that point at one particular tmux server. :class:`ServerConnection` is that pair +as one frozen value, and it is the only place in libtmux where either is +computed -- :meth:`libtmux.Server.cmd`, :meth:`libtmux.Server.raise_if_dead` and +:func:`libtmux.neo.fetch_objs` all read their flags from here. + +:meth:`ServerConnection.resolve_bin` is the single door to a tmux binary path: it +memoizes :func:`shutil.which` and raises +:exc:`~libtmux.exc.TmuxCommandNotFound` when tmux is absent, so no engine ships +an unguarded ``shutil.which("tmux")`` of its own. +""" + +from __future__ import annotations + +import shutil +import typing as t +from dataclasses import dataclass, field + +from libtmux import exc + +if t.TYPE_CHECKING: + import pathlib + from collections.abc import Sequence + + +class _BinaryResolver: + """Memoized tmux-binary resolution and ``tmux -V`` probe. + + Owned by a :class:`ServerConnection`; never constructed by engines. Holding + the mutable cache here keeps :class:`ServerConnection` a frozen, comparable + value. + """ + + __slots__ = ("_declared", "_resolved", "_version", "_version_probed") + + def __init__(self, tmux_bin: str | None = None) -> None: + self._declared = tmux_bin + self._resolved: str | None = None + self._version: str | None = None + self._version_probed = False + + def resolve(self) -> str: + """Return the tmux binary path, memoized for this connection. + + An explicit binary wins. Otherwise :func:`shutil.which` walks ``$PATH`` + once and the answer is cached. A *failure* is not cached, so a tmux + installed after the miss is picked up. + """ + if self._declared is not None: + return self._declared + if self._resolved is None: + resolved = shutil.which("tmux") + if resolved is None: + raise exc.TmuxCommandNotFound + self._resolved = resolved + return self._resolved + + def version(self) -> str | None: + """Return the tmux version string, memoized; ``None`` when unknowable. + + ``None`` (missing binary, unparseable output) lets version resolution + degrade to "assume latest" rather than exploding. + """ + if not self._version_probed: + self._version_probed = True + # Imported here, not at module scope: libtmux.common's tmux_cmd + # dispatches through this package, so a module-level import would + # close an import cycle. + from libtmux.common import get_version + + try: + self._version = str(get_version(self.resolve())) + except exc.LibTmuxException: + self._version = None + return self._version + + +@dataclass(frozen=True) +class ServerConnection: + """Which tmux binary, and which tmux server, an engine talks to. + + Attributes + ---------- + tmux_bin : str or None + An explicit tmux binary. ``None`` means "resolve from ``$PATH``", which + :meth:`resolve_bin` does once and memoizes. + args : tuple[str, ...] + Connection flags placed before the tmux subcommand (e.g. ``("-Lwork",)``). + _resolver : _BinaryResolver + Memoized resolver for the binary path and tmux version. Built in + ``__post_init__``; excluded from equality, hashing and :func:`repr`. + + Examples + -------- + The default connection targets the ambient tmux server: + + >>> ServerConnection() + ServerConnection(tmux_bin=None, args=()) + + :meth:`from_server` reads the flags off a live :class:`libtmux.Server`: + + >>> conn = ServerConnection.from_server(server) + >>> conn.args[0].startswith(("-L", "-S")) + True + + It duck-types, so any object with the same attributes works: + + >>> import types + >>> ServerConnection.from_server( + ... types.SimpleNamespace(socket_name="work", colors=256) + ... ) + ServerConnection(tmux_bin=None, args=('-2', '-Lwork')) + + :meth:`argv` prepends the binary and the flags to a command: + + >>> ServerConnection.of(tmux_bin="tmux", args=("-Lwork",)).argv( + ... "kill-window", "-t", "@1" + ... ) + ('tmux', '-Lwork', 'kill-window', '-t', '@1') + """ + + tmux_bin: str | None = None + args: tuple[str, ...] = () + _resolver: _BinaryResolver = field( + init=False, + repr=False, + compare=False, + hash=False, + ) + + def __post_init__(self) -> None: + """Normalize *args* and build the connection's binary resolver. + + Examples + -------- + >>> ServerConnection(args=["-Lwork"]).args + ('-Lwork',) + """ + object.__setattr__(self, "args", tuple(self.args)) + object.__setattr__(self, "_resolver", _BinaryResolver(self.tmux_bin)) + + @classmethod + def of( + cls, + tmux_bin: str | pathlib.Path | None = None, + args: Sequence[str] = (), + ) -> ServerConnection: + """Build a connection, stringifying a :class:`pathlib.Path` binary. + + Parameters + ---------- + tmux_bin : str or pathlib.Path, optional + Explicit tmux binary. + args : Sequence[str] + Connection flags. + + Returns + ------- + ServerConnection + The connection. + + Examples + -------- + >>> import pathlib + >>> ServerConnection.of(pathlib.Path("/usr/bin/tmux")).tmux_bin + '/usr/bin/tmux' + >>> ServerConnection.of(args=["-L", "test"]).args + ('-L', 'test') + """ + return cls( + tmux_bin=str(tmux_bin) if tmux_bin is not None else None, + args=tuple(args), + ) + + @classmethod + def from_server(cls, server: t.Any) -> ServerConnection: + """Build the connection a live :class:`libtmux.Server` talks over. + + Flags are emitted in tmux's documented order of significance and in the + order :meth:`libtmux.Server.cmd` has always emitted them: color depth, + ``-f`` config file, ``-S`` socket path, ``-L`` socket name. + + Parameters + ---------- + server : typing.Any + Any object exposing ``socket_name``, ``socket_path``, + ``config_file``, ``colors`` and ``tmux_bin``. Missing attributes are + treated as unset. + + Returns + ------- + ServerConnection + The connection. + + Raises + ------ + :exc:`~libtmux.exc.UnknownColorOption` + ``colors`` is truthy but is neither ``256`` nor ``88``. + + Examples + -------- + >>> import types + >>> ServerConnection.from_server( + ... types.SimpleNamespace(socket_path="/tmp/s", config_file="/tmp/c") + ... ) + ServerConnection(tmux_bin=None, args=('-f/tmp/c', '-S/tmp/s')) + + >>> from libtmux import exc + >>> try: + ... ServerConnection.from_server(types.SimpleNamespace(colors=16)) + ... except exc.UnknownColorOption as e: + ... print(e) + Server.colors must equal 88 or 256 + """ + args: list[str] = [] + + colors = getattr(server, "colors", None) + if colors: + if colors == 256: + args.append("-2") + elif colors == 88: + args.append("-8") + else: + raise exc.UnknownColorOption + + if getattr(server, "config_file", None): + args.append(f"-f{server.config_file}") + if getattr(server, "socket_path", None): + args.append(f"-S{server.socket_path}") + if getattr(server, "socket_name", None): + args.append(f"-L{server.socket_name}") + + return cls.of(tmux_bin=getattr(server, "tmux_bin", None), args=args) + + @property + def is_unconfigured(self) -> bool: + """Whether this connection names no server and no binary of its own. + + An unconfigured connection targets whichever tmux server ``tmux`` would + reach with no flags. :attr:`Server.engine ` reads + this to decide whether an injected engine should adopt the server's + connection: an engine that already names a server is left alone, and one + that names none is bound, so it cannot silently dispatch to the ambient + server. + + Returns + ------- + bool + + Examples + -------- + >>> ServerConnection().is_unconfigured + True + >>> ServerConnection.of(args=("-Lwork",)).is_unconfigured + False + >>> ServerConnection.of(tmux_bin="/usr/bin/tmux").is_unconfigured + False + """ + return not self.args and self.tmux_bin is None + + def resolve_bin(self) -> str: + """Return the tmux binary path (memoized). + + Returns + ------- + str + Path to tmux. + + Raises + ------ + :exc:`~libtmux.exc.TmuxCommandNotFound` + tmux is not on ``$PATH`` and none was declared. + + Examples + -------- + >>> ServerConnection.of(tmux_bin="/usr/bin/tmux").resolve_bin() + '/usr/bin/tmux' + """ + return self._resolver.resolve() + + def tmux_version(self) -> str | None: + """Return this connection's tmux version (memoized), or ``None``. + + Probes ``tmux -V`` once. Answers ``None`` when the binary is missing or + its version cannot be parsed, so a caller rendering version-gated argv + can fall back to assuming the newest tmux rather than failing. + + Returns + ------- + str or None + The version, e.g. ``"3.4"``. + + Examples + -------- + >>> ServerConnection().tmux_version() is not None + True + + The probe is memoized, so repeated reads cost one ``tmux -V``: + + >>> conn = ServerConnection() + >>> conn.tmux_version() == conn.tmux_version() + True + """ + return self._resolver.version() + + def argv(self, *args: str, tmux_bin: str | None = None) -> tuple[str, ...]: + """Render a full command line: binary, connection flags, then *args*. + + Parameters + ---------- + *args : str + The tmux subcommand and its arguments. + tmux_bin : str, optional + Override this connection's binary for one command. + + Returns + ------- + tuple[str, ...] + The full argv. + + Examples + -------- + >>> ServerConnection.of("tmux", ("-Lwork",)).argv("list-sessions") + ('tmux', '-Lwork', 'list-sessions') + >>> ServerConnection.of("tmux").argv("list-sessions", tmux_bin="/opt/tmux") + ('/opt/tmux', 'list-sessions') + """ + return (tmux_bin or self.resolve_bin(), *self.args, *args) diff --git a/src/libtmux/engines/subprocess.py b/src/libtmux/engines/subprocess.py new file mode 100644 index 0000000000..886dded420 --- /dev/null +++ b/src/libtmux/engines/subprocess.py @@ -0,0 +1,312 @@ +"""The default engine: one ``fork``/``exec`` of the tmux CLI per command. + +Mirrors the output handling libtmux has always had -- ``backslashreplace`` +decoding, trailing-blank stripping on stdout, blank filtering on stderr. A +tmux-side failure comes back as data (nonzero ``returncode`` plus ``stderr``); +only a missing binary raises. +""" + +from __future__ import annotations + +import logging +import shlex +import subprocess +import typing as t + +from libtmux import exc +from libtmux.engines.base import CommandResult +from libtmux.engines.connection import ServerConnection + +if t.TYPE_CHECKING: + import pathlib + from collections.abc import Sequence + + from libtmux.engines.base import CommandRequest + +logger = logging.getLogger(__name__) + + +class SubprocessEngine: + """Execute tmux commands by forking the tmux CLI binary. + + Parameters + ---------- + connection : ServerConnection, optional + The tmux binary and connection flags to dispatch through. Defaults to + the ambient tmux server on ``$PATH``. + + Examples + -------- + >>> from libtmux.engines import CommandRequest, SubprocessEngine + >>> engine = SubprocessEngine.for_server(server) + >>> engine.run(CommandRequest.from_args("display-message", "-p", "hi")).stdout + ('hi',) + """ + + def __init__(self, connection: ServerConnection | None = None) -> None: + self._conn = connection if connection is not None else ServerConnection() + + @classmethod + def of( + cls, + tmux_bin: str | pathlib.Path | None = None, + server_args: Sequence[str] = (), + ) -> SubprocessEngine: + """Build an engine from a binary path and raw connection flags. + + Parameters + ---------- + tmux_bin : str or pathlib.Path, optional + Explicit tmux binary; resolved from ``$PATH`` when ``None``. + server_args : Sequence[str] + Connection flags, e.g. ``("-Lwork",)``. + + Returns + ------- + SubprocessEngine + The engine. + + Examples + -------- + >>> SubprocessEngine.of(server_args=["-Lwork"]).server_args + ('-Lwork',) + """ + return cls(ServerConnection.of(tmux_bin, server_args)) + + def with_connection(self, connection: ServerConnection) -> SubprocessEngine: + """Return an equivalent engine dispatching over *connection*. + + Engines are immutable with respect to their connection, so this returns + a new engine rather than rebinding this one. + :attr:`Server.engine ` calls it to bind an engine + that names no server of its own. + + Parameters + ---------- + connection : ServerConnection + The connection the returned engine dispatches over. + + Returns + ------- + SubprocessEngine + A new engine; this one is left untouched. + + Examples + -------- + >>> from libtmux.engines import ServerConnection + >>> engine = SubprocessEngine() + >>> engine.server_args + () + >>> engine.with_connection(ServerConnection.of(args=("-Lwork",))).server_args + ('-Lwork',) + >>> engine.server_args + () + """ + return type(self)(connection) + + @classmethod + def for_server(cls, server: t.Any) -> SubprocessEngine: + """Build an engine bound to a live :class:`libtmux.Server`'s socket. + + Parameters + ---------- + server : typing.Any + Any object shaped like a :class:`libtmux.Server`. + + Returns + ------- + SubprocessEngine + An engine reaching the same tmux server as the object API. + + Examples + -------- + >>> SubprocessEngine.for_server(server).server_args[0].startswith("-L") + True + """ + return cls(ServerConnection.from_server(server)) + + @property + def connection(self) -> ServerConnection: + """The tmux binary + connection flags this engine dispatches through. + + Returns + ------- + ServerConnection + The connection. + + Examples + -------- + >>> SubprocessEngine.of("tmux").connection.tmux_bin + 'tmux' + """ + return self._conn + + @property + def tmux_bin(self) -> str | None: + """The explicitly configured tmux binary, if any. + + Returns + ------- + str or None + The declared binary; ``None`` when resolved from ``$PATH``. + + Examples + -------- + >>> SubprocessEngine.of("/usr/bin/tmux").tmux_bin + '/usr/bin/tmux' + """ + return self._conn.tmux_bin + + @property + def server_args(self) -> tuple[str, ...]: + """Connection flags placed before every tmux subcommand. + + Returns + ------- + tuple[str, ...] + The flags. + + Examples + -------- + >>> SubprocessEngine.of(server_args=("-Ltest",)).server_args + ('-Ltest',) + """ + return self._conn.args + + def tmux_version(self) -> str | None: + """Report the tmux version this engine dispatches to (memoized). + + Satisfies :class:`~libtmux.engines.base.SupportsTmuxVersion`. + + Returns + ------- + str or None + The version, or ``None`` when the binary is missing or unparseable. + + Examples + -------- + >>> SubprocessEngine.for_server(server).tmux_version() is not None + True + """ + return self._conn.tmux_version() + + def command_line(self, request: CommandRequest) -> tuple[str, ...]: + r"""Return the full argv *request* would run as, without running it. + + Parameters + ---------- + request : CommandRequest + The command. + + Returns + ------- + tuple[str, ...] + Binary, connection flags, then the encoded command argv. + + Examples + -------- + >>> from libtmux.engines import CommandRequest + >>> SubprocessEngine.of("tmux", ("-Lwork",)).command_line( + ... CommandRequest.from_args("send-keys", "echo hi") + ... ) + ('tmux', '-Lwork', 'send-keys', 'echo hi') + """ + return self._conn.argv(*request.args, tmux_bin=request.tmux_bin) + + def run(self, request: CommandRequest) -> CommandResult: + """Execute one tmux command via :mod:`subprocess` and return its result. + + Parameters + ---------- + request : CommandRequest + The command. + + Returns + ------- + CommandResult + Structured output, carrying the :class:`subprocess.Popen` that ran. + + Raises + ------ + :exc:`~libtmux.exc.TmuxCommandNotFound` + The tmux binary is missing or not executable. + + Examples + -------- + >>> from libtmux.engines import CommandRequest + >>> engine = SubprocessEngine.for_server(server) + >>> engine.run(CommandRequest.from_args("has-session", "-t", "nope")).returncode + 1 + """ + cmd = self.command_line(request) + + try: + process = subprocess.Popen( + cmd, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + encoding="utf-8", + errors="backslashreplace", + ) + stdout, stderr = process.communicate() + returncode = process.returncode + except FileNotFoundError: + raise exc.TmuxCommandNotFound from None + except Exception: + logger.error( # noqa: TRY400 + "tmux subprocess failed", + extra={"tmux_cmd": shlex.join(cmd)}, + ) + raise + + stdout_lines = stdout.split("\n") + while stdout_lines and stdout_lines[-1] == "": + stdout_lines.pop() + + result = CommandResult( + cmd=cmd, + stdout=tuple(stdout_lines), + stderr=tuple(line for line in stderr.split("\n") if line), + returncode=returncode, + process=process, + ) + if logger.isEnabledFor(logging.DEBUG): + logger.debug( + "tmux subprocess completed", + extra={ + "tmux_cmd": shlex.join(cmd), + "tmux_subcommand": request.subcommand, + "tmux_exit_code": returncode, + "tmux_stdout_len": len(result.stdout), + "tmux_stderr_len": len(result.stderr), + }, + ) + return result + + def run_batch(self, requests: Sequence[CommandRequest]) -> list[CommandResult]: + """Execute each request in order, one fork per command. + + Parameters + ---------- + requests : Sequence[CommandRequest] + Commands to run. + + Returns + ------- + list[CommandResult] + One result per request, in order. + + Examples + -------- + >>> from libtmux.engines import CommandRequest + >>> results = SubprocessEngine.for_server(server).run_batch( + ... [ + ... CommandRequest.from_args("display-message", "-p", "one"), + ... CommandRequest.from_args("display-message", "-p", "two"), + ... ] + ... ) + >>> [result.stdout[0] for result in results] + ['one', 'two'] + """ + return [self.run(request) for request in requests] diff --git a/src/libtmux/neo.py b/src/libtmux/neo.py index 98ece86fa5..feb8bb0e47 100644 --- a/src/libtmux/neo.py +++ b/src/libtmux/neo.py @@ -1098,17 +1098,7 @@ def fetch_objs( tmux_version = str(get_version(tmux_bin=server.tmux_bin)) _fields, format_string = get_output_format(list_cmd, tmux_version) - cmd_args: list[str | int] = [] - - if server.socket_name: - cmd_args.insert(0, f"-L{server.socket_name}") - if server.socket_path: - cmd_args.insert(0, f"-S{server.socket_path}") - - tmux_cmds = [ - *cmd_args, - list_cmd, - ] + tmux_cmds: list[str | int] = [list_cmd] if list_extra_args is not None and isinstance(list_extra_args, Iterable): tmux_cmds.extend(list(list_extra_args)) @@ -1130,10 +1120,7 @@ def fetch_objs( }, ) - proc = tmux_cmd( - *tmux_cmds, - tmux_bin=server.tmux_bin, - ) + proc = tmux_cmd(*tmux_cmds, engine=server.engine) raise_if_stderr(proc, list_cmd) diff --git a/src/libtmux/server.py b/src/libtmux/server.py index e650557c34..1d66023dda 100644 --- a/src/libtmux/server.py +++ b/src/libtmux/server.py @@ -10,7 +10,6 @@ import logging import os import pathlib -import shutil import subprocess import typing as t import warnings @@ -21,6 +20,9 @@ from libtmux.client import Client from libtmux.common import get_version, has_gte_version, raise_if_stderr, tmux_cmd from libtmux.constants import OptionScope +from libtmux.engines.base import CommandRequest, SupportsConnection +from libtmux.engines.connection import ServerConnection +from libtmux.engines.subprocess import SubprocessEngine from libtmux.hooks import HooksMixin from libtmux.neo import fetch_objs, get_output_format, parse_output from libtmux.pane import Pane @@ -43,6 +45,7 @@ from typing_extensions import Self from libtmux._internal.types import StrPath + from libtmux.engines.base import TmuxEngine DashLiteral: TypeAlias = t.Literal["-"] @@ -108,6 +111,10 @@ class Server( on_init : callable, optional socket_name_factory : callable, optional tmux_bin : str or pathlib.Path, optional + engine : :class:`~libtmux.engines.base.TmuxEngine`, optional + Executor every tmux command runs through. Defaults to + :class:`~libtmux.engines.subprocess.SubprocessEngine` bound to this + server's :attr:`connection`. Examples -------- @@ -167,6 +174,17 @@ class Server( tmux_bin: str | None = None """Custom path to tmux binary. Falls back to ``shutil.which("tmux")``.""" + _engine: TmuxEngine | None = None + """Caller-supplied executor, or ``None`` for the default subprocess engine.""" + _default_engine: SubprocessEngine | None = None + """Lazily built default engine, rebuilt whenever :attr:`connection` changes.""" + _connection: ServerConnection | None = None + """Cached connection, valid while :attr:`_connection_key` still matches.""" + _connection_key: tuple[t.Any, ...] | None = None + """Snapshot of the public connection attributes the cache was built from.""" + _adopted_engine: tuple[ServerConnection, TmuxEngine] | None = None + """Injected engine rebound to :attr:`connection`, with the connection it used.""" + def __init__( self, socket_name: str | None = None, @@ -176,10 +194,16 @@ def __init__( on_init: t.Callable[[Server], None] | None = None, socket_name_factory: t.Callable[[], str] | None = None, tmux_bin: str | pathlib.Path | None = None, + engine: TmuxEngine | None = None, **kwargs: t.Any, ) -> None: EnvironmentMixin.__init__(self, "-g") self.tmux_bin = str(tmux_bin) if tmux_bin is not None else None + self._engine = engine + self._default_engine = None + self._adopted_engine = None + self._connection = None + self._connection_key = None self._windows: list[WindowDict] = [] self._panes: list[PaneDict] = [] @@ -199,6 +223,115 @@ def __init__( if on_init is not None: on_init(self) + @property + def connection(self) -> ServerConnection: + """Return the tmux binary and connection flags this server dispatches on. + + :attr:`socket_name`, :attr:`socket_path`, :attr:`config_file`, + :attr:`colors` and :attr:`tmux_bin` are public and writable, and + :meth:`__eq__` reads two of them, so a connection captured once at + construction would silently keep pointing at the old socket after a + write. The connection is therefore *derived*, and cached against a + snapshot of exactly those five attributes: reassigning any of them + invalidates the cache on the next command, while an unchanged server + keeps one memoized :func:`shutil.which` lookup for its whole life. + + Returns + ------- + :class:`~libtmux.engines.connection.ServerConnection` + Flags in the order tmux receives them: color depth, ``-f``, ``-S``, + ``-L``. + + Raises + ------ + :exc:`~libtmux.exc.UnknownColorOption` + :attr:`colors` is set to something other than ``256`` or ``88``. + + Examples + -------- + >>> tmux = Server(socket_name="engine_conn_docs") + >>> tmux.connection.args + ('-Lengine_conn_docs',) + + A later write is picked up: + + >>> tmux.socket_name = "engine_conn_docs_moved" + >>> tmux.connection.args + ('-Lengine_conn_docs_moved',) + + .. versionadded:: 0.63 + """ + key = ( + self.socket_name, + None if self.socket_path is None else str(self.socket_path), + self.config_file, + self.colors, + self.tmux_bin, + ) + if self._connection is None or self._connection_key != key: + self._connection = ServerConnection.from_server(self) + self._connection_key = key + return self._connection + + @property + def engine(self) -> TmuxEngine: + """Return the executor every tmux command on this server runs through. + + With no ``engine=``, a + :class:`~libtmux.engines.subprocess.SubprocessEngine` is built from + :attr:`connection` and rebuilt whenever that connection changes. + + A caller-supplied ``engine=`` that already names a tmux server is + returned untouched. One that names none -- a bare + ``SubprocessEngine()`` -- *adopts* this server's :attr:`connection`, + because returning it untouched would dispatch to whichever server a + flagless ``tmux`` reaches rather than to this one. Engines with no + connection at all, such as in-memory fakes, are always returned + untouched. + + Returns + ------- + :class:`~libtmux.engines.base.TmuxEngine` + The engine. + + Examples + -------- + >>> from libtmux.engines import SubprocessEngine + >>> isinstance(server.engine, SubprocessEngine) + True + + An injected engine that names no server adopts this one's socket: + + >>> tmux = Server(socket_name="engine_adopt_docs", engine=SubprocessEngine()) + >>> tmux.engine.server_args + ('-Lengine_adopt_docs',) + + An engine that names a server keeps it: + + >>> pinned = SubprocessEngine.of(server_args=("-Lelsewhere",)) + >>> Server(socket_name="engine_adopt_docs", engine=pinned).engine.server_args + ('-Lelsewhere',) + + .. versionadded:: 0.63 + """ + connection = self.connection + engine = self._engine + if engine is not None: + if not isinstance(engine, SupportsConnection): + return engine + if connection.is_unconfigured or not engine.connection.is_unconfigured: + return engine + adopted = self._adopted_engine + if adopted is None or adopted[0] is not connection: + adopted = (connection, engine.with_connection(connection)) + self._adopted_engine = adopted + return adopted[1] + default = self._default_engine + if default is None or default.connection is not connection: + default = SubprocessEngine(connection) + self._default_engine = default + return default + @classmethod def from_env(cls, env: t.Mapping[str, str] | None = None) -> Server: """Return the tmux server this process's pane is attached to. @@ -317,22 +450,9 @@ def raise_if_dead(self) -> None: ... print(type(e)) """ - resolved = self.tmux_bin or shutil.which("tmux") - if resolved is None: - raise exc.TmuxCommandNotFound - - cmd_args: list[str] = ["list-sessions"] - if self.socket_name: - cmd_args.insert(0, f"-L{self.socket_name}") - if self.socket_path: - cmd_args.insert(0, f"-S{self.socket_path}") - if self.config_file: - cmd_args.insert(0, f"-f{self.config_file}") - - try: - subprocess.check_call([resolved, *cmd_args]) - except FileNotFoundError: - raise exc.TmuxCommandNotFound from None + result = self.engine.run(CommandRequest.from_args("list-sessions")) + if result.returncode != 0: + raise subprocess.CalledProcessError(result.returncode, list(result.cmd)) # # Command @@ -386,29 +506,19 @@ def cmd( Notes ----- + Dispatches through :attr:`Server.engine`; the connection flags come + from :attr:`Server.connection`, so this method and every other tmux + call on this server target the same socket. + .. versionchanged:: 0.8 Renamed from ``.tmux`` to ``.cmd``. """ - svr_args: list[str | int] = [cmd] - cmd_args: list[str | int] = [] - if self.socket_name: - svr_args.insert(0, f"-L{self.socket_name}") - if self.socket_path: - svr_args.insert(0, f"-S{self.socket_path}") - if self.config_file: - svr_args.insert(0, f"-f{self.config_file}") - if self.colors: - if self.colors == 256: - svr_args.insert(0, "-2") - elif self.colors == 88: - svr_args.insert(0, "-8") - else: - raise exc.UnknownColorOption - - cmd_args = ["-t", str(target), *args] if target is not None else [*args] + cmd_args: list[str | int] = ( + ["-t", str(target), *args] if target is not None else [*args] + ) - return tmux_cmd(*svr_args, *cmd_args, tmux_bin=self.tmux_bin) + return tmux_cmd(cmd, *cmd_args, engine=self.engine) @property def attached_sessions(self) -> list[Session]: diff --git a/tests/test_engines.py b/tests/test_engines.py new file mode 100644 index 0000000000..cfd9f0fa1d --- /dev/null +++ b/tests/test_engines.py @@ -0,0 +1,289 @@ +"""Tests for :mod:`libtmux.engines`, the tmux command execution seam.""" + +from __future__ import annotations + +import subprocess +import typing as t + +import pytest + +from libtmux import exc +from libtmux.common import tmux_cmd +from libtmux.engines import ( + CommandRequest, + CommandResult, + ServerConnection, + SubprocessEngine, + SupportsCommandLine, + TmuxEngine, +) +from libtmux.neo import fetch_objs +from libtmux.server import Server + +if t.TYPE_CHECKING: + from collections.abc import Sequence + + from libtmux.session import Session + + +class CannedEngine: + """An in-memory engine: records requests, replays canned stdout. + + Satisfies :class:`~libtmux.engines.base.TmuxEngine` structurally, without + inheritance and without a tmux binary. + """ + + def __init__(self, stdout: Sequence[str] = ()) -> None: + self.requests: list[CommandRequest] = [] + self._stdout = tuple(stdout) + + def run(self, request: CommandRequest) -> CommandResult: + """Record *request* and return the canned result.""" + self.requests.append(request) + return CommandResult( + cmd=("canned-tmux", *request.args), + stdout=self._stdout, + ) + + def run_batch(self, requests: Sequence[CommandRequest]) -> list[CommandResult]: + """Run each request in order.""" + return [self.run(request) for request in requests] + + +def test_canned_engine_satisfies_protocol() -> None: + """A plain class with run/run_batch is a TmuxEngine.""" + assert isinstance(CannedEngine(), TmuxEngine) + assert not isinstance(CannedEngine(), SupportsCommandLine) + + +def test_server_drives_injected_engine_without_tmux() -> None: + """``Server(engine=...)`` routes ``cmd()`` through the injected engine. + + No tmux fixture: the point is that an injected engine never forks tmux, so + the canned stdout is what ``Server.cmd`` returns. + """ + engine = CannedEngine(stdout=("$9",)) + server = Server(socket_name="canned_never_started", engine=engine) + + proc = server.cmd("new-session", "-P", "-F#{session_id}") + + assert proc.stdout == ["$9"] + assert proc.returncode == 0 + assert proc.cmd == ["canned-tmux", "new-session", "-P", "-F#{session_id}"] + assert [request.args for request in engine.requests] == [ + ("new-session", "-P", "-F#{session_id}"), + ] + assert server.engine is engine + + +def test_injected_engine_receives_target_flag() -> None: + """``target=`` is rendered into the request, not the connection.""" + engine = CannedEngine() + server = Server(socket_name="canned_target", engine=engine) + + server.cmd("kill-window", target="@3") + + assert engine.requests[0].args == ("kill-window", "-t", "@3") + + +def test_process_raises_on_engine_without_subprocess() -> None: + """``.process`` is unavailable when no OS process was forked.""" + server = Server(socket_name="canned_process", engine=CannedEngine()) + proc = server.cmd("list-sessions") + + with pytest.raises(exc.LibTmuxException): + _ = proc.process + + +class ForeignResult(t.NamedTuple): + """A result shaped like :class:`CommandResult` but of another type. + + An out-of-tree engine has no reason to import libtmux's result class, and + :class:`~libtmux.engines.base.TmuxEngine` never says it must. ``process`` is + absent here on purpose: it is the one field no protocol declares. + """ + + cmd: tuple[str, ...] + stdout: tuple[str, ...] = () + stderr: tuple[str, ...] = () + returncode: int = 0 + + +class ForeignResultEngine: + """An engine returning a result type libtmux does not own.""" + + def run(self, request: CommandRequest) -> t.Any: + """Return a structurally-compatible result of a foreign type.""" + return ForeignResult(cmd=("foreign-tmux", *request.args), stdout=("$7",)) + + def run_batch(self, requests: Sequence[CommandRequest]) -> list[t.Any]: + """Run each request in order.""" + return [self.run(request) for request in requests] + + +def test_server_drives_engine_returning_a_foreign_result() -> None: + """An engine may return any structurally-compatible result, not only ours. + + ``TmuxEngine`` is structural, so an out-of-tree engine that never imports + :class:`CommandResult` still qualifies. Reading ``process`` off such a + result must degrade to the documented exception rather than raising + :exc:`AttributeError` from inside dispatch. + """ + server = Server(socket_name="foreign_result", engine=ForeignResultEngine()) + + proc = server.cmd("new-session", "-P", "-F#{session_id}") + + assert proc.stdout == ["$7"] + assert proc.returncode == 0 + assert proc.cmd == ["foreign-tmux", "new-session", "-P", "-F#{session_id}"] + with pytest.raises(exc.LibTmuxException): + _ = proc.process + + +def test_process_is_popen_under_default_engine(session: Session) -> None: + """``.process`` reads exactly as it did before the seam existed.""" + proc = session.server.cmd("display-message", "-p", "hi") + + assert isinstance(proc.process, subprocess.Popen) + assert proc.process.returncode == 0 + + +def test_connection_follows_socket_name_mutation() -> None: + """A post-construction write to ``socket_name`` changes the flags used. + + ``Server.socket_name`` is public and writable, so the connection is derived + per command rather than captured at construction. + """ + server = Server(socket_name="mutation_before") + assert server.connection.args == ("-Lmutation_before",) + first = server.connection + + server.socket_name = "mutation_after" + + assert server.connection.args == ("-Lmutation_after",) + assert server.connection is not first + assert server.cmd("has-session", "-t", "nothing").cmd[1] == "-Lmutation_after" + + +def test_connection_is_cached_while_unchanged(server: Server) -> None: + """An untouched server reuses one connection, and so one binary lookup.""" + assert server.connection is server.connection + assert server.engine is server.engine + + +def test_default_engine_rebuilt_after_mutation() -> None: + """The default engine is rebuilt when the connection it wraps changes.""" + server = Server(socket_name="engine_rebuild_before") + first = server.engine + + server.socket_name = "engine_rebuild_after" + second = server.engine + + assert first is not second + assert isinstance(second, SubprocessEngine) + assert second.server_args == ("-Lengine_rebuild_after",) + + +def test_injected_engine_survives_mutation() -> None: + """An injected engine is user-owned: libtmux never swaps it out.""" + engine = CannedEngine() + server = Server(socket_name="injected_before", engine=engine) + + server.socket_name = "injected_after" + + assert server.engine is engine + + +class ArgvRecordingEngine: + """Render argv against a real connection, record it, run nothing. + + Lets a test read the command line each dispatch path *would* have used, + without a tmux server and without special-casing any one path. + """ + + def __init__(self, connection: ServerConnection) -> None: + self.connection = connection + self.command_lines: list[tuple[str, ...]] = [] + + def run(self, request: CommandRequest) -> CommandResult: + """Record the rendered argv and return an empty success.""" + cmd = (self.connection.tmux_bin or "tmux", *self.connection.args, *request.args) + self.command_lines.append(cmd) + return CommandResult(cmd=cmd) + + def run_batch(self, requests: Sequence[CommandRequest]) -> list[CommandResult]: + """Run each request in order.""" + return [self.run(request) for request in requests] + + +def test_flag_builders_agree() -> None: + """cmd(), raise_if_dead() and fetch_objs() emit identical flags. + + All three paths formerly built ``-L``/``-S``/``-f``/``-2`` themselves, from + three different rules. They now read one + :class:`~libtmux.engines.connection.ServerConnection`. + """ + attrs: dict[str, t.Any] = { + "socket_name": "flag_agreement", + "config_file": "/dev/null", + "colors": 256, + } + expected = Server(**attrs).connection.args + assert expected == ("-2", "-f/dev/null", "-Lflag_agreement") + + engine = ArgvRecordingEngine(Server(**attrs).connection) + server = Server(**attrs, engine=engine) + + server.cmd("list-sessions") + server.raise_if_dead() + fetch_objs(server=server, list_cmd="list-sessions") + + assert len(engine.command_lines) == 3 + assert {line[1 : 1 + len(expected)] for line in engine.command_lines} == {expected} + + +def test_unknown_color_raises_on_every_path() -> None: + """An unknown ``colors`` value raises, matching ``Server.cmd``'s contract.""" + server = Server(socket_name="bad_colors") + server.colors = 16 + + with pytest.raises(exc.UnknownColorOption): + server.cmd("list-sessions") + with pytest.raises(exc.UnknownColorOption): + server.raise_if_dead() + with pytest.raises(exc.UnknownColorOption): + fetch_objs(server=server, list_cmd="list-sessions") + + +def test_command_request_rejects_nul() -> None: + """NUL cannot survive tmux's C-string argv.""" + with pytest.raises(ValueError, match="NUL"): + CommandRequest.from_args("display-message", "a\0b") + + +def test_connection_from_server_duck_types() -> None: + """``from_server`` reads any object with the five connection attributes.""" + conn = ServerConnection.from_server( + Server(socket_path="/tmp/spike-sock", config_file="/tmp/spike-conf"), + ) + assert conn.args == ("-f/tmp/spike-conf", "-S/tmp/spike-sock") + + +def test_missing_binary_raises_tmux_command_not_found() -> None: + """A declared-but-absent tmux binary raises, on every path.""" + engine = SubprocessEngine.of("/nonexistent/tmux") + with pytest.raises(exc.TmuxCommandNotFound): + engine.run(CommandRequest.from_args("list-sessions")) + with pytest.raises(exc.TmuxCommandNotFound): + tmux_cmd("list-sessions", tmux_bin="/nonexistent/tmux") + + +def test_run_batch_preserves_order(session: Session) -> None: + """``run_batch`` returns one result per request, in order.""" + results = SubprocessEngine.for_server(session.server).run_batch( + [ + CommandRequest.from_args("display-message", "-p", "a"), + CommandRequest.from_args("display-message", "-p", "b"), + ], + ) + assert [result.stdout[0] for result in results] == ["a", "b"] From be3af1663efbde33773001cc74f4c735e7e15c97 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 16 Aug 2026 07:00:10 -0500 Subject: [PATCH 02/16] Engines(fix[adoption]): Bind binary-only engines why: A custom tmux_bin names a program, not a server. An engine built with one and no -L/-S was treated as already knowing its server, so it was left unbound and every command reached whichever tmux server a flagless dispatch finds -- the silent ambient dispatch adoption exists to prevent. what: - Add ServerConnection.names_server, asking whether a connection carries connection flags of its own; the engine side of adoption reads it instead of is_unconfigured, which keeps its server-side meaning of "carries nothing at all" - Bind the server's flags onto such an engine while preserving the binary it was built with - Document the binary-is-not-a-server rule on Server.engine and in the CHANGES deliverable prose - Cover both adoption directions plus the two cases that already held, so a single-predicate regression cannot pass --- CHANGES | 4 ++- src/libtmux/engines/connection.py | 45 +++++++++++++++++++++---- src/libtmux/server.py | 35 +++++++++++++++----- tests/test_engines.py | 55 +++++++++++++++++++++++++++++++ 4 files changed, 122 insertions(+), 17 deletions(-) diff --git a/CHANGES b/CHANGES index b8b6d18522..e00ebb59d5 100644 --- a/CHANGES +++ b/CHANGES @@ -86,7 +86,9 @@ thing you can now replace. An engine that names no tmux server of its own adopts the server's connection, so injecting one into a socket-scoped {class}`~libtmux.Server` cannot silently -dispatch to the ambient tmux server. Engines that name a server keep it. +dispatch to the ambient tmux server. Engines that name a server keep it. A +custom `tmux_bin` selects a program rather than a server, so an engine carrying +only one adopts the server's flags and keeps its own binary. {class}`~libtmux.engines.connection.ServerConnection` is now the single place the tmux binary and the `-L`/`-S`/`-f`/`-2`/`-8` flags are computed; three diff --git a/src/libtmux/engines/connection.py b/src/libtmux/engines/connection.py index d2043838ec..318c4619e1 100644 --- a/src/libtmux/engines/connection.py +++ b/src/libtmux/engines/connection.py @@ -237,14 +237,12 @@ def from_server(cls, server: t.Any) -> ServerConnection: @property def is_unconfigured(self) -> bool: - """Whether this connection names no server and no binary of its own. + """Whether this connection carries nothing at all -- no flags, no binary. - An unconfigured connection targets whichever tmux server ``tmux`` would - reach with no flags. :attr:`Server.engine ` reads - this to decide whether an injected engine should adopt the server's - connection: an engine that already names a server is left alone, and one - that names none is bound, so it cannot silently dispatch to the ambient - server. + :attr:`Server.engine ` reads this on the + *server's* side of adoption: a server that names neither a socket nor a + binary has nothing to bind onto an injected engine, so it leaves the + engine alone. Returns ------- @@ -261,6 +259,39 @@ def is_unconfigured(self) -> bool: """ return not self.args and self.tmux_bin is None + @property + def names_server(self) -> bool: + """Whether this connection carries connection flags of its own. + + :attr:`Server.engine ` reads this on the + *engine's* side of adoption: an engine that already carries flags knows + which tmux server it talks to and is left alone, while one that carries + none is bound to the server's flags so it cannot silently dispatch to + the ambient server. + + :attr:`tmux_bin` deliberately does not count. It selects which tmux + *program* to exec, which says nothing about which server that program + connects to -- a custom binary with no ``-L``/``-S`` reaches the same + ambient server as the stock one. + + Returns + ------- + bool + + Examples + -------- + >>> ServerConnection.of(args=("-Lwork",)).names_server + True + >>> ServerConnection().names_server + False + + A binary is a program, not a server: + + >>> ServerConnection.of(tmux_bin="/usr/bin/tmux").names_server + False + """ + return bool(self.args) + def resolve_bin(self) -> str: """Return the tmux binary path (memoized). diff --git a/src/libtmux/server.py b/src/libtmux/server.py index 1d66023dda..ae25ae60ed 100644 --- a/src/libtmux/server.py +++ b/src/libtmux/server.py @@ -281,13 +281,15 @@ def engine(self) -> TmuxEngine: :class:`~libtmux.engines.subprocess.SubprocessEngine` is built from :attr:`connection` and rebuilt whenever that connection changes. - A caller-supplied ``engine=`` that already names a tmux server is - returned untouched. One that names none -- a bare - ``SubprocessEngine()`` -- *adopts* this server's :attr:`connection`, - because returning it untouched would dispatch to whichever server a - flagless ``tmux`` reaches rather than to this one. Engines with no - connection at all, such as in-memory fakes, are always returned - untouched. + A caller-supplied ``engine=`` carrying connection flags of its own + already names a tmux server, and is returned untouched. One carrying + none -- a bare ``SubprocessEngine()`` -- *adopts* this server's + :attr:`connection`, because returning it untouched would dispatch to + whichever server a flagless ``tmux`` reaches rather than to this one. A + ``tmux_bin`` does not count as naming a server: it selects which tmux + program to exec, so an engine carrying only a binary adopts this + server's flags and keeps its own binary. Engines with no connection at + all, such as in-memory fakes, are always returned untouched. Returns ------- @@ -312,6 +314,14 @@ def engine(self) -> TmuxEngine: >>> Server(socket_name="engine_adopt_docs", engine=pinned).engine.server_args ('-Lelsewhere',) + A binary names no server, so an engine carrying only one still binds, + and keeps that binary: + + >>> custom = SubprocessEngine.of(tmux_bin="/nonexistent/tmux") + >>> bound = Server(socket_name="engine_adopt_docs", engine=custom).engine + >>> bound.server_args, bound.tmux_bin + (('-Lengine_adopt_docs',), '/nonexistent/tmux') + .. versionadded:: 0.63 """ connection = self.connection @@ -319,11 +329,18 @@ def engine(self) -> TmuxEngine: if engine is not None: if not isinstance(engine, SupportsConnection): return engine - if connection.is_unconfigured or not engine.connection.is_unconfigured: + engine_connection = engine.connection + if connection.is_unconfigured or engine_connection.names_server: return engine adopted = self._adopted_engine if adopted is None or adopted[0] is not connection: - adopted = (connection, engine.with_connection(connection)) + target = connection + if engine_connection.tmux_bin is not None: + target = ServerConnection.of( + engine_connection.tmux_bin, + connection.args, + ) + adopted = (connection, engine.with_connection(target)) self._adopted_engine = adopted return adopted[1] default = self._default_engine diff --git a/tests/test_engines.py b/tests/test_engines.py index cfd9f0fa1d..ad27b6ddd0 100644 --- a/tests/test_engines.py +++ b/tests/test_engines.py @@ -194,6 +194,61 @@ def test_injected_engine_survives_mutation() -> None: assert server.engine is engine +def test_engine_carrying_only_a_binary_still_adopts_the_socket() -> None: + """A tmux binary names a *program*, not a server, so the socket still binds. + + Left unbound, such an engine runs `` list-sessions`` with no + ``-L``, reaching whichever server a flagless tmux finds rather than this + one -- the silent ambient dispatch adoption exists to prevent. + """ + engine = SubprocessEngine.of(tmux_bin="/nonexistent/tmux") + server = Server(socket_name="bin_only_adopts", engine=engine) + + adopted = server.engine + + assert isinstance(adopted, SubprocessEngine) + assert adopted.command_line(CommandRequest.from_args("list-sessions")) == ( + "/nonexistent/tmux", + "-Lbin_only_adopts", + "list-sessions", + ) + + +def test_adoption_keeps_the_engines_own_binary() -> None: + """Adoption takes the server's flags without discarding the engine's binary.""" + engine = SubprocessEngine.of(tmux_bin="/nonexistent/tmux") + server = Server(socket_name="bin_kept", tmux_bin="/other/tmux", engine=engine) + + adopted = server.engine + + assert isinstance(adopted, SubprocessEngine) + assert adopted.tmux_bin == "/nonexistent/tmux" + assert adopted.server_args == ("-Lbin_kept",) + + +def test_server_binary_reaches_an_engine_that_declares_none() -> None: + """An engine with no binary of its own still inherits the server's.""" + server = Server( + socket_name="bin_inherited", + tmux_bin="/other/tmux", + engine=SubprocessEngine(), + ) + + adopted = server.engine + + assert isinstance(adopted, SubprocessEngine) + assert adopted.tmux_bin == "/other/tmux" + assert adopted.server_args == ("-Lbin_inherited",) + + +def test_engine_naming_a_server_is_left_alone() -> None: + """Connection flags of the engine's own win over the server's.""" + engine = SubprocessEngine.of(server_args=("-Lelsewhere",)) + server = Server(socket_name="not_elsewhere", engine=engine) + + assert server.engine is engine + + class ArgvRecordingEngine: """Render argv against a real connection, record it, run nothing. From 9275a87d3006fbe3c1bcdec26872b90519dc61e8 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 16 Aug 2026 07:03:37 -0500 Subject: [PATCH 03/16] Server(fix[raise_if_dead]): Attach tmux's message why: The engine captures tmux's stderr instead of letting it reach the terminal, and the raise then discarded it, so a caller was left with an exit code and no way to recover what tmux said -- strictly less than the message the terminal used to show. what: - Pass the captured stdout and stderr to CalledProcessError, matching what CompletedProcess.check_returncode raises - Say so in the docstring and prove it in the doctest - Assert the socket name reaches the exception, which holds across both wordings tmux uses for an unreachable server --- src/libtmux/server.py | 16 ++++++++++++++-- tests/test_engines.py | 16 ++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/libtmux/server.py b/src/libtmux/server.py index ae25ae60ed..8053605759 100644 --- a/src/libtmux/server.py +++ b/src/libtmux/server.py @@ -452,24 +452,36 @@ def is_alive(self) -> bool: def raise_if_dead(self) -> None: """Raise if server not connected. + The engine captures tmux's diagnostic rather than letting it reach the + terminal, so it rides on the exception as + :attr:`~subprocess.CalledProcessError.stderr` -- otherwise an exit code + is all the caller ever sees of why the server is unreachable. + Raises ------ :exc:`exc.TmuxCommandNotFound` When the tmux binary cannot be found or executed. :class:`subprocess.CalledProcessError` When the tmux server is not running (non-zero exit from - ``list-sessions``). + ``list-sessions``), carrying tmux's own message. >>> tmux = Server(socket_name="no_exist") >>> try: ... tmux.raise_if_dead() ... except Exception as e: ... print(type(e)) + ... print("no_exist" in e.stderr) + True """ result = self.engine.run(CommandRequest.from_args("list-sessions")) if result.returncode != 0: - raise subprocess.CalledProcessError(result.returncode, list(result.cmd)) + raise subprocess.CalledProcessError( + result.returncode, + list(result.cmd), + output="\n".join(result.stdout), + stderr="\n".join(result.stderr), + ) # # Command diff --git a/tests/test_engines.py b/tests/test_engines.py index ad27b6ddd0..7fce41125f 100644 --- a/tests/test_engines.py +++ b/tests/test_engines.py @@ -310,6 +310,22 @@ def test_unknown_color_raises_on_every_path() -> None: fetch_objs(server=server, list_cmd="list-sessions") +def test_raise_if_dead_carries_tmuxs_message() -> None: + """The dead-server diagnostic rides on the exception instead of vanishing. + + The engine captures tmux's stderr rather than letting it reach the + terminal, so dropping it would leave the caller with an exit code and + nothing to explain it. + """ + server = Server(socket_name="raise_if_dead_message") + + with pytest.raises(subprocess.CalledProcessError) as excinfo: + server.raise_if_dead() + + assert excinfo.value.stderr is not None + assert "raise_if_dead_message" in excinfo.value.stderr + + def test_command_request_rejects_nul() -> None: """NUL cannot survive tmux's C-string argv.""" with pytest.raises(ValueError, match="NUL"): From 740526e77a718907724110861eaa3c053935a88a Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 16 Aug 2026 12:33:10 -0500 Subject: [PATCH 04/16] Engines(fix[dispatch]): Reject an async engine at dispatch why: TmuxEngine and SupportsCommandLine are runtime_checkable Protocols, so isinstance() checks attribute names only -- never signatures, never async-ness. An engine with `async def run` satisfied them, was accepted by Server(engine=...), and failed on the first command with `AttributeError: 'coroutine' object has no attribute 'cmd'`, naming neither the engine nor the mismatch. An `async def command_line` failed the same way, one line earlier, whenever DEBUG logging was on. what: - Guard every engine capability in one place, _guard_sync(), reached through the typed _dispatch_run() and _dispatch_command_line() wrappers, so a mistyped call site is a mypy error rather than a runtime AttributeError - Collapse raise_if_dead onto self.cmd(), deleting the second dispatch site rather than guarding it twice - Reject a declared-async member before calling it, so the common shape never creates a coroutine; test the returned value too, since a plain def can still hand one back - Close a coroutine that did get created -- safe while unstarted, and suppressed against BaseException so a hostile awaitable cannot replace the diagnostic. Never cancel a Task or Future: one bound to another thread's loop would not receive it, and one shared with another awaiter would lose its result - Let AsyncEngineMismatch escape the list-accessor leniency; a misconfigured engine is not a tmux failure and must not read as "no sessions" - Add exc.AsyncEngineMismatch, naming the engine and the method, and document it on cmd() for Server, Session, Window and Pane - Show the failure as a runnable example in docs/topics/engines.md An eagerly-started Task (3.12+) has already run its body before run() returns; the guard reports it but cannot undo it. Nothing dispatches run_batch in-tree, so it gets no guard. --- docs/topics/engines.md | 25 +++++ src/libtmux/common.py | 151 ++++++++++++++++++++++++- src/libtmux/exc.py | 61 +++++++++++ src/libtmux/pane.py | 5 + src/libtmux/server.py | 34 +++++- src/libtmux/session.py | 5 + src/libtmux/window.py | 10 ++ tests/test_engines.py | 244 +++++++++++++++++++++++++++++++++++++++++ 8 files changed, 529 insertions(+), 6 deletions(-) diff --git a/docs/topics/engines.md b/docs/topics/engines.md index 29aebc121c..3100072a25 100644 --- a/docs/topics/engines.md +++ b/docs/topics/engines.md @@ -81,6 +81,31 @@ binary, a dropped connection — raises: is no base class to inherit — any object with `run()` and `run_batch()` is an engine. +`run()` and the optional `command_line()` must be synchronous: libtmux +dispatches every command from ordinary, non-`async` code and cannot await a +coroutine. Because {class}`~libtmux.engines.base.TmuxEngine` is checked by name +only, an `async def run()` satisfies it and would otherwise fail much later, +with a bare `AttributeError` naming neither the engine nor the mismatch: + +```python +>>> from libtmux.engines import CommandResult +>>> from libtmux.server import Server + +>>> class AsyncEngine: +... async def run(self, request): +... return CommandResult(cmd=("tmux", *request.args)) +... def run_batch(self, requests): +... return [self.run(request) for request in requests] + +>>> Server(engine=AsyncEngine()).cmd("display-message", "-p", "#S") +Traceback (most recent call last): + ... +libtmux.exc.AsyncEngineMismatch: AsyncEngine.run() returned an awaitable: ... +``` + +Await such an engine from your own async code instead, or write a synchronous +`run()`. + Here is a complete one that runs nothing, records everything, and answers from a canned script. Hand it to a server and no tmux process is involved: diff --git a/src/libtmux/common.py b/src/libtmux/common.py index f496e84dde..c1832c051b 100644 --- a/src/libtmux/common.py +++ b/src/libtmux/common.py @@ -7,7 +7,9 @@ from __future__ import annotations +import contextlib import functools +import inspect import logging import re import shlex @@ -23,7 +25,7 @@ import subprocess from collections.abc import Callable - from .engines.base import TmuxEngine + from .engines.base import CommandResult, TmuxEngine logger = logging.getLogger(__name__) @@ -283,6 +285,141 @@ def raise_if_stderr(proc: tmux_cmd, subcommand: str) -> None: ) +def _guard_sync( + engine: object, + member: Callable[[CommandRequest], t.Any], + method: str, + request: CommandRequest, +) -> t.Any: + """Call one synchronous engine capability, guarding the result. + + The single call site every engine capability -- ``run()`` and the + optional ``command_line()`` -- is invoked through. + :class:`~libtmux.engines.base.TmuxEngine` and + :class:`~libtmux.engines.base.SupportsCommandLine` are + :func:`~typing.runtime_checkable` :class:`typing.Protocol` classes, so + ``isinstance()`` accepts an engine on attribute *names* alone -- never + signatures, never async-ness -- and an ``async def run`` (or ``async def + command_line``) engine passes structurally and reaches here. Routing + every dispatch through this one function means the guard below only has + to be written once: a call site added later inherits it instead of + needing its own copy. + + Parameters + ---------- + engine : object + The engine the capability belongs to; named in the error. + member : :class:`~collections.abc.Callable` + The already-resolved bound method to invoke. + method : str + Its name, ``"run"`` or ``"command_line"``, for the error message. + request : CommandRequest + Forwarded as the sole positional argument. + + Returns + ------- + typing.Any + Whatever *method* returned. Never an awaitable. + + Raises + ------ + :exc:`~libtmux.exc.AsyncEngineMismatch` + *method* returned an awaitable instead of the value its protocol + promises. + + Notes + ----- + Declared-``async def`` members are rejected *before* the call, so the + common shape never creates a coroutine at all and nothing is left to warn + about. That check cannot be complete on its own -- CPython says as much in + :mod:`unittest.async_case`, whose case 3 is a "regular ``def`` that + returns an awaitable object" -- so the value is tested too. + + A coroutine that did get created is closed, which is safe precisely + because it has never been started: :c:func:`gen_close` on a frame still in + ``FRAME_CREATED`` clears it without running a line of the body, and + ``"coroutine ... was never awaited"`` is only warned for a frame still in + that state at collection. Closing is best-effort -- guarded against + :class:`BaseException`, since :exc:`asyncio.CancelledError` is not an + :class:`Exception` -- so a hostile awaitable cannot replace the + diagnostic with an error of its own. + + Only genuine coroutines are closed. A :class:`asyncio.Task` or + :class:`asyncio.Future` is dropped untouched: one bound to another + thread's event loop silently fails to receive + :meth:`~asyncio.Task.cancel` (that needs ``loop.call_soon_threadsafe``), + and cancelling one shared with another awaiter would destroy that + awaiter's result. An eager-started ``Task`` (3.12+) has already run its + body synchronously before ``run()`` returned, so nothing here could have + prevented that side effect either way. + """ + if inspect.iscoroutinefunction(member): + raise exc.AsyncEngineMismatch(engine, method) + + result = member(request) + if inspect.isawaitable(result): + if inspect.iscoroutine(result): + with contextlib.suppress(BaseException): + result.close() + raise exc.AsyncEngineMismatch(engine, method) + return result + + +def _dispatch_run(engine: TmuxEngine, request: CommandRequest) -> CommandResult: + """Run one command through *engine*, guarding the result. + + Parameters + ---------- + engine : TmuxEngine + The engine to dispatch through. + request : CommandRequest + The command. + + Returns + ------- + CommandResult + Whatever ``run()`` returned. Never an awaitable. + + Raises + ------ + :exc:`~libtmux.exc.AsyncEngineMismatch` + ``run`` is asynchronous. + """ + return t.cast( + "CommandResult", + _guard_sync(engine, engine.run, "run", request), + ) + + +def _dispatch_command_line( + engine: SupportsCommandLine, + request: CommandRequest, +) -> tuple[str, ...]: + """Render *request*'s argv through *engine*, guarding the result. + + Parameters + ---------- + engine : SupportsCommandLine + The engine to ask. + request : CommandRequest + The command. + + Returns + ------- + tuple[str, ...] + The argv. Never an awaitable. + + Raises + ------ + :exc:`~libtmux.exc.AsyncEngineMismatch` + ``command_line`` is asynchronous. + """ + return t.cast( + "tuple[str, ...]", + _guard_sync(engine, engine.command_line, "command_line", request), + ) + + class tmux_cmd: """Run any :term:`tmux(1)` command, returning list-shaped output. @@ -314,6 +451,14 @@ class tmux_cmd: returncode : int tmux exit code. + Raises + ------ + :exc:`~libtmux.exc.AsyncEngineMismatch` + *engine* is asynchronous -- its ``run()`` (or ``command_line()``, + while rendering a DEBUG log line) handed back an awaitable, which + this synchronous dispatch cannot await. Both calls route through + :func:`_guard_sync`, the one place this is checked. + Examples -------- Create a new session, check for error: @@ -356,7 +501,7 @@ def __init__( "tmux command dispatched", extra={ "tmux_cmd": shlex.join( - runner.command_line(request) + _dispatch_command_line(runner, request) if isinstance(runner, SupportsCommandLine) else request.args, ), @@ -364,7 +509,7 @@ def __init__( }, ) - result = runner.run(request) + result = _dispatch_run(runner, request) self.cmd = list(result.cmd) self.returncode = result.returncode diff --git a/src/libtmux/exc.py b/src/libtmux/exc.py index 57bb06102f..0dd2326cd0 100644 --- a/src/libtmux/exc.py +++ b/src/libtmux/exc.py @@ -99,6 +99,67 @@ class TmuxCommandNotFound(LibTmuxException): """Application binary for tmux not found.""" +class AsyncEngineMismatch(LibTmuxException): + """A synchronous dispatch path received an engine call that returned an awaitable. + + :class:`~libtmux.engines.base.TmuxEngine` and + :class:`~libtmux.engines.base.SupportsCommandLine` are + :func:`typing.runtime_checkable` :class:`typing.Protocol` classes, which + check attribute *names* only -- never signatures or async-ness. An engine + declared with ``async def run`` (or ``async def command_line``) still + satisfies ``isinstance(engine, TmuxEngine)`` and reaches + :class:`~libtmux.common.tmux_cmd`, whose dispatch is synchronous and + cannot await it. + + Raised from what the call actually returned, not from inspecting the + method beforehand, so it also catches a method that is not itself + declared ``async`` but still hands back an awaitable -- an engine that + wraps its coroutine in an :class:`asyncio.Task` or :class:`asyncio.Future` + before returning it. + + Parameters + ---------- + engine : object + The engine instance whose method returned an awaitable. + method : str + Name of the method that returned it -- ``"run"`` or + ``"command_line"``. + *args : object + Forwarded to :class:`LibTmuxException`. + + Examples + -------- + >>> from libtmux import exc + >>> class AsyncEngine: + ... async def run(self, request): ... + ... async def run_batch(self, requests): ... + >>> print( # doctest: +NORMALIZE_WHITESPACE + ... exc.AsyncEngineMismatch(AsyncEngine(), "run") + ... ) + AsyncEngine.run() returned an awaitable: libtmux dispatches tmux commands + synchronously and cannot await it. Await this engine directly from your + own async code, or pass a synchronous engine. + + It is part of the :exc:`LibTmuxException` hierarchy: + + >>> issubclass(exc.AsyncEngineMismatch, exc.LibTmuxException) + True + + .. versionadded:: 0.63 + """ + + def __init__(self, engine: object, method: str, *args: object) -> None: + self.engine = engine + self.method = method + msg = ( + f"{type(engine).__name__}.{method}() returned an awaitable: " + "libtmux dispatches tmux commands synchronously and cannot " + "await it. Await this engine directly from your own async " + "code, or pass a synchronous engine." + ) + super().__init__(msg, *args) + + class NotInsideTmux(LibTmuxException): """Raised when the process is not running inside a tmux pane. diff --git a/src/libtmux/pane.py b/src/libtmux/pane.py index e0c2f59619..8c4cce6a93 100644 --- a/src/libtmux/pane.py +++ b/src/libtmux/pane.py @@ -336,6 +336,11 @@ def cmd( Returns ------- :meth:`server.cmd` + + Raises + ------ + :exc:`~libtmux.exc.AsyncEngineMismatch` + The server's engine is asynchronous; see :meth:`Server.cmd`. """ if target is None: target = self.pane_id diff --git a/src/libtmux/server.py b/src/libtmux/server.py index 8053605759..3758dce2bf 100644 --- a/src/libtmux/server.py +++ b/src/libtmux/server.py @@ -20,7 +20,7 @@ from libtmux.client import Client from libtmux.common import get_version, has_gte_version, raise_if_stderr, tmux_cmd from libtmux.constants import OptionScope -from libtmux.engines.base import CommandRequest, SupportsConnection +from libtmux.engines.base import SupportsConnection from libtmux.engines.connection import ServerConnection from libtmux.engines.subprocess import SubprocessEngine from libtmux.hooks import HooksMixin @@ -457,10 +457,19 @@ def raise_if_dead(self) -> None: :attr:`~subprocess.CalledProcessError.stderr` -- otherwise an exit code is all the caller ever sees of why the server is unreachable. + Dispatches through :meth:`Server.cmd`, the same path every other tmux + command on this server takes, rather than calling :attr:`Server.engine` + directly -- one dispatch site instead of two. + Raises ------ + :exc:`~libtmux.exc.UnknownColorOption` + :attr:`colors` is set to something other than ``256`` or ``88``. :exc:`exc.TmuxCommandNotFound` When the tmux binary cannot be found or executed. + :exc:`~libtmux.exc.AsyncEngineMismatch` + An injected engine's ``run()`` returned an awaitable; this path + cannot await it. :class:`subprocess.CalledProcessError` When the tmux server is not running (non-zero exit from ``list-sessions``), carrying tmux's own message. @@ -474,11 +483,11 @@ def raise_if_dead(self) -> None: True """ - result = self.engine.run(CommandRequest.from_args("list-sessions")) + result = self.cmd("list-sessions") if result.returncode != 0: raise subprocess.CalledProcessError( result.returncode, - list(result.cmd), + result.cmd, output="\n".join(result.stdout), stderr="\n".join(result.stderr), ) @@ -533,6 +542,13 @@ def cmd( ------- :class:`common.tmux_cmd` + Raises + ------ + :exc:`~libtmux.exc.AsyncEngineMismatch` + The engine is asynchronous -- its ``run()`` (or ``command_line()``, + while rendering a DEBUG log line) returned an awaitable, which this + synchronous dispatch cannot await. + Notes ----- Dispatches through :attr:`Server.engine`; the connection flags come @@ -2551,12 +2567,18 @@ def sessions(self) -> QueryList[Session]: missing socket, a permission error, or a subprocess failure. To distinguish "no sessions" from "tmux unreachable", call :meth:`Server.is_alive` or :meth:`Server.raise_if_dead`. + + :exc:`~libtmux.exc.AsyncEngineMismatch` is not a tmux failure -- it + means the injected engine cannot be dispatched synchronously at all -- + so it is not part of that leniency and always propagates. """ try: sessions: list[Session] = [ Session(server=self, **obj) for obj in fetch_objs(server=self, list_cmd="list-sessions") ] + except exc.AsyncEngineMismatch: + raise except exc.LibTmuxException: return QueryList([]) return QueryList(sessions) @@ -2613,6 +2635,10 @@ def clients(self) -> QueryList[Client]: distinguish "no clients attached" from "tmux unreachable", call :meth:`Server.is_alive` or :meth:`Server.raise_if_dead`. + :exc:`~libtmux.exc.AsyncEngineMismatch` is not a tmux failure -- it + means the injected engine cannot be dispatched synchronously at all -- + so it is not part of that leniency and always propagates. + Returns ------- :class:`~libtmux._internal.query_list.QueryList` of :class:`Client` @@ -2629,6 +2655,8 @@ def clients(self) -> QueryList[Client]: Client(server=self, **obj) for obj in fetch_objs(server=self, list_cmd="list-clients") ] + except exc.AsyncEngineMismatch: + raise except exc.LibTmuxException: return QueryList([]) return QueryList(clients) diff --git a/src/libtmux/session.py b/src/libtmux/session.py index 4277052a37..3a325cdec7 100644 --- a/src/libtmux/session.py +++ b/src/libtmux/session.py @@ -444,6 +444,11 @@ def cmd( ------- :meth:`server.cmd` + Raises + ------ + :exc:`~libtmux.exc.AsyncEngineMismatch` + The server's engine is asynchronous; see :meth:`Server.cmd`. + Notes ----- .. versionchanged:: 0.34 diff --git a/src/libtmux/window.py b/src/libtmux/window.py index b57db99692..d25cb4c90a 100644 --- a/src/libtmux/window.py +++ b/src/libtmux/window.py @@ -317,6 +317,9 @@ def linked_sessions(self) -> QueryList[Session]: holders takes two list commands total, independent of how many there are. If either listing fails, the result is empty. + :exc:`~libtmux.exc.AsyncEngineMismatch` is not a listing failure -- + it means the server's engine cannot be dispatched synchronously at + all -- so it always propagates instead. Returns ------- @@ -363,6 +366,8 @@ def linked_sessions(self) -> QueryList[Session]: server=self.server, list_cmd="list-sessions", ) + except exc.AsyncEngineMismatch: + raise except exc.LibTmuxException: return QueryList([]) @@ -490,6 +495,11 @@ def cmd( Returns ------- :meth:`server.cmd` + + Raises + ------ + :exc:`~libtmux.exc.AsyncEngineMismatch` + The server's engine is asynchronous; see :meth:`Server.cmd`. """ if target is None: target = self.window_id diff --git a/tests/test_engines.py b/tests/test_engines.py index 7fce41125f..fccd2232ae 100644 --- a/tests/test_engines.py +++ b/tests/test_engines.py @@ -2,6 +2,9 @@ from __future__ import annotations +import asyncio +import gc +import logging import subprocess import typing as t @@ -358,3 +361,244 @@ def test_run_batch_preserves_order(session: Session) -> None: ], ) assert [result.stdout[0] for result in results] == ["a", "b"] + + +class AsyncEngine: + """Structurally a :class:`TmuxEngine`, but both methods are ``async def``. + + ``TmuxEngine`` checks attribute names only, so this still satisfies + ``isinstance(..., TmuxEngine)``. + """ + + async def run(self, request: CommandRequest) -> CommandResult: + """Never actually awaited by libtmux; dispatch must reject this.""" + return CommandResult(cmd=("tmux", *request.args)) + + async def run_batch( + self, + requests: Sequence[CommandRequest], + ) -> list[CommandResult]: + """Unused by any in-tree dispatch path.""" + return [CommandResult(cmd=("tmux", *r.args)) for r in requests] + + +def _async_engine() -> TmuxEngine: + """Hand back an :class:`AsyncEngine`, typed as a plain ``TmuxEngine``. + + ``AsyncEngine`` does not satisfy ``TmuxEngine`` *statically* -- its + methods return ``Coroutine``, not the protocol's declared return types -- + which is exactly what makes the bug this module tests real: a type + checker would reject it, but ``isinstance()`` at runtime does not. The + cast documents that gap instead of hiding it behind a broader type on + ``AsyncEngine`` itself. + """ + return t.cast("TmuxEngine", AsyncEngine()) + + +def test_async_engine_run_raises_named_error() -> None: + """``run()`` returning an awaitable raises ``AsyncEngineMismatch``. + + Not ``AttributeError`` from treating a coroutine as a + :class:`CommandResult`. + """ + server = Server(socket_name="async_engine_run", engine=_async_engine()) + + with pytest.raises(exc.AsyncEngineMismatch): + server.cmd("list-sessions") + + +def test_async_engine_raise_if_dead_raises_the_same_error() -> None: + """``raise_if_dead()`` shares :meth:`Server.cmd`'s single dispatch site. + + It no longer calls ``self.engine.run()`` on its own, so it inherits the + guard instead of needing a second copy of it. + """ + server = Server(socket_name="async_engine_dead", engine=_async_engine()) + + with pytest.raises(exc.AsyncEngineMismatch): + server.raise_if_dead() + + +def test_async_engine_fetch_objs_raises_the_same_error() -> None: + """:func:`~libtmux.neo.fetch_objs` dispatches through the same guard.""" + server = Server(socket_name="async_engine_fetch_objs", engine=_async_engine()) + + with pytest.raises(exc.AsyncEngineMismatch): + fetch_objs(server=server, list_cmd="list-sessions") + + +class AsyncCommandLineEngine: + """A synchronous ``run()`` paired with an asynchronous ``command_line()``. + + Isolates the DEBUG-log-only dispatch site: ``command_line()`` is only + ever called to render the log line in :class:`tmux_cmd`, never to build + the actual result. + """ + + def run(self, request: CommandRequest) -> CommandResult: + """Behave like an ordinary synchronous engine.""" + return CommandResult(cmd=("tmux", *request.args)) + + def run_batch(self, requests: Sequence[CommandRequest]) -> list[CommandResult]: + """Run each request in order.""" + return [self.run(r) for r in requests] + + async def command_line(self, request: CommandRequest) -> tuple[str, ...]: + """Return the argv, the one async method on an otherwise sync engine.""" + return ("tmux", *request.args) + + +def test_async_command_line_raises_named_error_under_debug_logging( + caplog: pytest.LogCaptureFixture, +) -> None: + """``command_line()`` only runs when DEBUG logging is enabled. + + Previously this bypassed the guard entirely and raised + ``TypeError: 'coroutine' object is not iterable`` from ``shlex.join``. + """ + server = Server( + socket_name="async_command_line", + engine=AsyncCommandLineEngine(), + ) + + with ( + caplog.at_level(logging.DEBUG, logger="libtmux.common"), + pytest.raises(exc.AsyncEngineMismatch), + ): + server.cmd("list-sessions") + + +@pytest.mark.parametrize("attr", ["sessions", "clients", "attached_sessions"]) +def test_async_engine_list_accessors_do_not_swallow_the_error(attr: str) -> None: + """``AsyncEngineMismatch`` is not a tmux failure, so it is not lenient here. + + :attr:`Server.sessions`, :attr:`Server.clients`, and + :attr:`Server.attached_sessions` return an empty + :class:`~libtmux._internal.query_list.QueryList` for an actual tmux + failure (no daemon, bad socket, permission error). An engine that cannot + be dispatched synchronously at all is a different kind of problem -- + surfacing it as "no sessions" would hide a broken engine behind a + misleading empty result. + """ + server = Server(socket_name=f"async_engine_{attr}", engine=_async_engine()) + + with pytest.raises(exc.AsyncEngineMismatch): + getattr(server, attr) + + +class HostileGetattrAwaitable: + """An awaitable that detonates on any ``close``/``cancel`` lookup. + + Cleanup that reached for those attributes would surface this object's + ``RuntimeError`` in place of the diagnostic, which is the failure mode + the guard's shape exists to avoid. + """ + + def __await__(self) -> t.Generator[None, None, None]: + """Satisfy :func:`inspect.isawaitable` without ever being awaited.""" + yield + + def __getattr__(self, name: str) -> t.Any: + """Raise for the cleanup lookups, ``AttributeError`` for the rest.""" + if name in {"close", "cancel"}: + msg = "cleanup lookup blew up" + raise RuntimeError(msg) + raise AttributeError(name) + + +class HostileCloseCoroutine: + """An awaitable whose ``close()`` raises a :class:`BaseException`. + + :exc:`asyncio.CancelledError` derives from :class:`BaseException`, not + :class:`Exception`, so an ``except Exception`` around cleanup would let + it escape and mask the diagnostic. + """ + + def __await__(self) -> t.Generator[None, None, None]: + """Satisfy :func:`inspect.isawaitable` without ever being awaited.""" + yield + + def close(self) -> None: + """Raise the exception an ``except Exception`` would not catch.""" + raise asyncio.CancelledError + + +def _engine_returning(value: t.Any) -> TmuxEngine: + """Build a sync engine whose ``run()`` hands back *value*.""" + + class Returns: + def run(self, request: CommandRequest) -> t.Any: + return value + + def run_batch(self, requests: Sequence[CommandRequest]) -> t.Any: + return [value for _ in requests] + + return t.cast("TmuxEngine", Returns()) + + +@pytest.mark.parametrize( + "awaitable", + [HostileGetattrAwaitable(), HostileCloseCoroutine()], + ids=["hostile-getattr", "cancelled-error-on-close"], +) +def test_hostile_awaitable_cannot_mask_the_mismatch(awaitable: t.Any) -> None: + """A hostile awaitable never replaces the diagnostic with its own error. + + Only genuine coroutines are closed, and that close is guarded against + :class:`BaseException`, so neither an exploding attribute lookup nor a + :exc:`asyncio.CancelledError` reaches the caller. + """ + server = Server( + socket_name="hostile_awaitable", engine=_engine_returning(awaitable) + ) + + with pytest.raises(exc.AsyncEngineMismatch): + server.cmd("list-sessions") + + +async def _never_awaited() -> None: + """Do nothing; this body must never run.""" + + +class ReturnsCoroutineEngine: + """A plain ``def`` engine that manufactures a coroutine anyway. + + The shape CPython documents as uncatchable by a callable-level check -- + ``run`` is not declared ``async``, so only its return value gives it away. + """ + + def run(self, request: CommandRequest) -> t.Any: + """Hand back an unstarted coroutine instead of a result.""" + return _never_awaited() + + def run_batch(self, requests: Sequence[CommandRequest]) -> t.Any: + """Hand back one unstarted coroutine per request.""" + return [_never_awaited() for _ in requests] + + +@pytest.mark.parametrize( + ("label", "engine_factory"), + [ + ("declared-async", _async_engine), + ("returns-coroutine", lambda: t.cast("TmuxEngine", ReturnsCoroutineEngine())), + ], +) +def test_no_never_awaited_warning_escapes( + label: str, + engine_factory: t.Callable[[], TmuxEngine], + recwarn: pytest.WarningsRecorder, +) -> None: + """Neither async shape leaves a ``coroutine ... was never awaited`` behind. + + A declared ``async def run`` is rejected before it is ever called, so no + coroutine is created. A plain ``def`` that manufactures one is caught from + its return value, and that coroutine is closed while still unstarted. + """ + server = Server(socket_name=f"warnfree_{label}", engine=engine_factory()) + + with pytest.raises(exc.AsyncEngineMismatch): + server.cmd("list-sessions") + + gc.collect() + + assert [w for w in recwarn.list if issubclass(w.category, RuntimeWarning)] == [] From c13869cabdf1f6b56315d6efa7a7ee2ba873d599 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sat, 22 Aug 2026 19:53:31 -0500 Subject: [PATCH 05/16] Engines(feat[base]): Count the commands an argv carries why: A caller measuring engine traffic cannot tell a request that ran one tmux command from one that inlined several into a single dispatch. The distinction lives in the argv -- a CommandSeparator marks a real boundary while a literal ";" is data -- and every engine already agrees on it through is_command_separator. Leaving the arithmetic to each observer invites them to count the encoded argv instead, where the separator has been flattened to a plain string and the inlining is invisible. what: - Add command_count beside is_command_separator, returning the separators plus one, with doctests covering a group and a literal semicolon - Export it from libtmux.engines --- src/libtmux/engines/__init__.py | 2 ++ src/libtmux/engines/base.py | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/libtmux/engines/__init__.py b/src/libtmux/engines/__init__.py index 01eaabe4c8..00c7d82a57 100644 --- a/src/libtmux/engines/__init__.py +++ b/src/libtmux/engines/__init__.py @@ -46,6 +46,7 @@ SupportsConnection, SupportsTmuxVersion, TmuxEngine, + command_count, is_command_separator, ) from libtmux.engines.connection import ServerConnection @@ -61,5 +62,6 @@ "SupportsConnection", "SupportsTmuxVersion", "TmuxEngine", + "command_count", "is_command_separator", ) diff --git a/src/libtmux/engines/base.py b/src/libtmux/engines/base.py index 36c727c9c7..224b56859b 100644 --- a/src/libtmux/engines/base.py +++ b/src/libtmux/engines/base.py @@ -89,6 +89,39 @@ def is_command_separator(token: str) -> bool: return type(token) is CommandSeparator and token == ";" +def command_count(argv: tuple[str, ...]) -> int: + """Return how many tmux commands a rendered *argv* runs. + + A command group is one argv carrying several commands, separated by + :class:`CommandSeparator`, so the count is the separators plus one. Callers + that measure engine traffic need this to tell a request that ran one tmux + command from a request that inlined several into a single dispatch. + + Parameters + ---------- + argv : tuple of str + A request's arguments, before any engine-specific encoding. + + Returns + ------- + int + + Examples + -------- + >>> command_count(("list-panes", "-a")) + 1 + >>> group = ("set-option", "-g", "@x", "1", CommandSeparator(";"), "show-options") + >>> command_count(group) + 2 + + A literal ``";"`` is data, not a boundary, so it does not add a command: + + >>> command_count(("send-keys", ";")) + 1 + """ + return sum(1 for token in argv if is_command_separator(token)) + 1 + + @dataclass(frozen=True) class CommandRequest: """A tmux command, ready for an engine to execute. From c51971321b883fa483107b09c1c9f75b1c0b6904 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sat, 22 Aug 2026 19:54:04 -0500 Subject: [PATCH 06/16] Engines(feat[instrumentation]): Add the observation seam why: Counting or tracing tmux traffic meant patching subprocess.Popen from outside, which under-reports any engine that does not fork and cannot see a command group at all. TmuxEngine is a protocol, so the honest place for observation is a decorator that satisfies the same protocol: a program that wants none of it constructs none of it and runs the code it ran before, with no guard on the hot path. SQLAlchemy pays a boolean check per call for its event registry and Django builds a context mapping even with no wrapper registered; both are shaped by having concrete connection classes. A protocol lets the wrapper substitute for the engine wherever one is accepted, and cost nothing where it is not. what: - Add Sink, a before/after/error observer surface matching the hooks OpenTelemetry and Sentry already target on SQLAlchemy, so an exporter written against one reads naturally here - Add CountingSink, reporting requests, tmux commands, the commands that rode inside another request's argv, and elapsed time - Count on request.args rather than the encoded argv, where an engine has flattened the separator and the inlining no longer shows - Add InstrumentedEngine, which forwards anything the protocol does not cover to the engine it wraps - Pin the substitution property as a test: the wrapper is a TmuxEngine --- src/libtmux/engines/__init__.py | 10 + src/libtmux/engines/instrumentation.py | 244 +++++++++++++++++++++++++ tests/test_engines_instrumentation.py | 198 ++++++++++++++++++++ 3 files changed, 452 insertions(+) create mode 100644 src/libtmux/engines/instrumentation.py create mode 100644 tests/test_engines_instrumentation.py diff --git a/src/libtmux/engines/__init__.py b/src/libtmux/engines/__init__.py index 00c7d82a57..bba2ae94c4 100644 --- a/src/libtmux/engines/__init__.py +++ b/src/libtmux/engines/__init__.py @@ -50,18 +50,28 @@ is_command_separator, ) from libtmux.engines.connection import ServerConnection +from libtmux.engines.instrumentation import ( + CountingSink, + InstrumentedEngine, + Sink, + instrument, +) from libtmux.engines.subprocess import SubprocessEngine __all__ = ( "CommandRequest", "CommandResult", "CommandSeparator", + "CountingSink", + "InstrumentedEngine", "ServerConnection", + "Sink", "SubprocessEngine", "SupportsCommandLine", "SupportsConnection", "SupportsTmuxVersion", "TmuxEngine", "command_count", + "instrument", "is_command_separator", ) diff --git a/src/libtmux/engines/instrumentation.py b/src/libtmux/engines/instrumentation.py new file mode 100644 index 0000000000..2294b98039 --- /dev/null +++ b/src/libtmux/engines/instrumentation.py @@ -0,0 +1,244 @@ +"""Observe engine traffic without paying for it when nobody is watching. + +Instrumentation here is **composed, not installed**. An +:class:`InstrumentedEngine` implements the same protocol as the engine it +wraps, so an uninstrumented program never constructs one and executes exactly +the code it executed before: no guard, no branch, no context object on the hot +path. + +That differs from how the SQL ecosystem solves this, and deliberately. +SQLAlchemy exposes an event registry and pays one boolean check per call; +Django folds wrappers around each execute and builds a context mapping even +when no wrapper is registered. Both are shaped by having concrete connection +classes. :class:`~libtmux.engines.base.TmuxEngine` is a protocol, so a +decorator substitutes for the real engine anywhere one is accepted, and costs +nothing where it is absent. + +The observer surface intentionally mirrors the one OpenTelemetry and Sentry +already target on SQLAlchemy -- a before hook, an after hook, and an error +hook -- so an exporter written against it needs no monkeypatching. + +Examples +-------- +Count what a run costs, without changing how it runs: + +>>> from libtmux.engines import CommandRequest, SubprocessEngine +>>> counts = CountingSink() +>>> engine = instrument(SubprocessEngine.for_server(server), counts) +>>> _ = engine.run(CommandRequest.from_args("show-options", "-g")) +>>> counts.requests, counts.tmux_commands, counts.inlined +(1, 1, 0) + +One request may carry several tmux commands. The extra ones rode along inside +an argv that spawned a single process, which is what ``inlined`` reports: + +>>> from libtmux.engines import CommandSeparator +>>> counts = CountingSink() +>>> engine = instrument(SubprocessEngine.for_server(server), counts) +>>> _ = engine.run( +... CommandRequest.from_args( +... "set-option", "-g", "@x", "1", CommandSeparator(";"), "show-options", "-g" +... ) +... ) +>>> counts.requests, counts.tmux_commands, counts.inlined +(1, 2, 1) +""" + +from __future__ import annotations + +import time +import typing as t + +from libtmux.engines.base import command_count + +if t.TYPE_CHECKING: + from collections.abc import Sequence + + from libtmux.engines.base import CommandRequest, CommandResult + +__all__ = [ + "CountingSink", + "InstrumentedEngine", + "Sink", + "instrument", +] + + +@t.runtime_checkable +class Sink(t.Protocol): + """An observer of engine traffic. + + The three methods mirror the hook names OpenTelemetry and Sentry attach to + on SQLAlchemy, so an exporter written for one reads naturally here. + + Whatever :meth:`before_command` returns is handed back to + :meth:`after_command` and :meth:`handle_error` as ``state``, which lets a + sink carry a span or a start time without keeping its own map. + """ + + def before_command(self, request: CommandRequest) -> t.Any: + """Observe a request about to run; return per-command state.""" + ... + + def after_command( + self, request: CommandRequest, result: CommandResult, state: t.Any + ) -> None: + """Observe a completed request.""" + ... + + def handle_error( + self, request: CommandRequest, error: BaseException, state: t.Any + ) -> None: + """Observe a request that raised. The error still propagates.""" + ... + + +class CountingSink: + """Accumulate how much tmux work passed through an engine. + + Attributes + ---------- + requests : int + Requests dispatched to the engine. + tmux_commands : int + tmux commands those requests carried, counting a command group as its + members rather than as one. + elapsed_ns : int + Total wall time spent inside the engine. + + Examples + -------- + >>> sink = CountingSink() + >>> sink.requests, sink.tmux_commands, sink.inlined + (0, 0, 0) + """ + + __slots__ = ("elapsed_ns", "requests", "tmux_commands") + + def __init__(self) -> None: + self.requests = 0 + self.tmux_commands = 0 + self.elapsed_ns = 0 + + @property + def inlined(self) -> int: + """Commands that rode inside another request's argv. + + Examples + -------- + >>> sink = CountingSink() + >>> sink.requests, sink.tmux_commands = 3, 5 + >>> sink.inlined + 2 + """ + return self.tmux_commands - self.requests + + def before_command(self, request: CommandRequest) -> int: + """Count the request and its commands, returning a start timestamp. + + Counting happens here, on ``request.args``, because an engine's own + encoding flattens :class:`~libtmux.engines.base.CommandSeparator` into + a plain string. An observer reading the encoded argv would report no + inlining. + """ + self.requests += 1 + self.tmux_commands += command_count(tuple(request.args)) + return time.perf_counter_ns() + + def after_command( + self, request: CommandRequest, result: CommandResult, state: t.Any + ) -> None: + """Add this command's duration to the total.""" + del request, result + self.elapsed_ns += time.perf_counter_ns() - state + + def handle_error( + self, request: CommandRequest, error: BaseException, state: t.Any + ) -> None: + """Charge a failed command's duration too.""" + del request, error + self.elapsed_ns += time.perf_counter_ns() - state + + +class InstrumentedEngine: + """Wrap a synchronous engine so sinks observe every command. + + Examples + -------- + >>> from libtmux.engines import CommandRequest, SubprocessEngine + >>> counts = CountingSink() + >>> engine = InstrumentedEngine(SubprocessEngine.for_server(server), counts) + >>> _ = engine.run_batch([CommandRequest.from_args("show-options", "-g")] * 3) + >>> counts.requests + 3 + """ + + __slots__ = ("_inner", "_sinks") + + def __init__(self, inner: t.Any, *sinks: Sink) -> None: + self._inner = inner + self._sinks = sinks + + @property + def inner(self) -> t.Any: + """The engine being observed.""" + return self._inner + + def __getattr__(self, name: str) -> t.Any: + """Forward anything the protocol does not cover to the inner engine.""" + return getattr(self._inner, name) + + def run(self, request: CommandRequest) -> CommandResult: + """Run one request, notifying every sink around it.""" + states = [sink.before_command(request) for sink in self._sinks] + try: + result: CommandResult = self._inner.run(request) + except BaseException as error: + for sink, state in zip(self._sinks, states, strict=True): + sink.handle_error(request, error, state) + raise + for sink, state in zip(self._sinks, states, strict=True): + sink.after_command(request, result, state) + return result + + def run_batch(self, requests: Sequence[CommandRequest]) -> list[CommandResult]: + """Run a batch, observing each request individually. + + The inner engine still decides how the batch is dispatched; only the + observation is per request. + """ + return [self.run(request) for request in requests] + + +def instrument(engine: t.Any, *sinks: Sink) -> t.Any: + """Wrap *engine* so *sinks* observe every command it runs. + + Parameters + ---------- + engine : object + Any engine satisfying :class:`~libtmux.engines.base.TmuxEngine`. + *sinks : Sink + Observers, notified in the order given. + + Returns + ------- + InstrumentedEngine + A stand-in implementing the same protocol as *engine*. + + Examples + -------- + >>> from libtmux.engines import SubprocessEngine + >>> type(instrument(SubprocessEngine.for_server(server), CountingSink())).__name__ + 'InstrumentedEngine' + + The wrapper forwards anything the protocol does not cover, so it stands in + wherever the engine did: + + >>> instrument(SubprocessEngine.for_server(server)).inner.__class__.__name__ + 'SubprocessEngine' + """ + # SPIKE: the async half (AsyncInstrumentedEngine, and dispatching on + # inspect.iscoroutinefunction(engine.run)) is deliberately absent -- this + # seam has no async engine protocol yet. When one lands, `instrument` grows + # the branch and the async wrapper joins it. + return InstrumentedEngine(engine, *sinks) diff --git a/tests/test_engines_instrumentation.py b/tests/test_engines_instrumentation.py new file mode 100644 index 0000000000..982fd7f617 --- /dev/null +++ b/tests/test_engines_instrumentation.py @@ -0,0 +1,198 @@ +"""Tests for observing engine traffic at the command execution seam.""" + +from __future__ import annotations + +import typing as t + +import pytest + +from libtmux.engines import ( + CommandRequest, + CommandSeparator, + CountingSink, + InstrumentedEngine, + Sink, + SubprocessEngine, + TmuxEngine, + command_count, + instrument, +) +from libtmux.engines.base import CommandResult + +if t.TYPE_CHECKING: + from collections.abc import Sequence + + from libtmux.server import Server + + +class _ExplodingEngine: + """An engine whose every command raises, to exercise the error hook.""" + + def run(self, request: CommandRequest) -> CommandResult: + msg = f"boom: {request.args[0]}" + raise RuntimeError(msg) + + def run_batch(self, requests: Sequence[CommandRequest]) -> list[CommandResult]: + return [self.run(request) for request in requests] + + +def test_instrumented_engine_satisfies_the_engine_protocol() -> None: + """A wrapper stands in wherever the engine it wraps was accepted. + + This is the property the whole design rests on: observation is a + substitution, so nothing downstream needs to know it happened. + """ + engine = InstrumentedEngine(_ExplodingEngine(), CountingSink()) + + assert isinstance(engine, TmuxEngine) + + +def test_counting_sink_separates_requests_from_tmux_commands(server: Server) -> None: + """A command group is one request carrying several tmux commands.""" + counts = CountingSink() + engine = instrument(SubprocessEngine.for_server(server), counts) + + engine.run(CommandRequest.from_args("show-options", "-g")) + engine.run( + CommandRequest.from_args( + "set-option", + "-g", + "@spike", + "1", + CommandSeparator(";"), + "show-options", + "-g", + ), + ) + + assert counts.requests == 2 + assert counts.tmux_commands == 3 + assert counts.inlined == 1 + + +def test_counting_happens_on_args_not_the_encoded_argv(server: Server) -> None: + """Inlining is only visible before an engine flattens the separator. + + ``CommandSeparator`` is a ``str`` subclass, so any encoding that renders + argv to plain strings erases the distinction. Counting on ``request.args`` + is what keeps the inlined figure meaningful. + """ + grouped = CommandRequest.from_args( + "set-option", + "-g", + "@spike", + "1", + CommandSeparator(";"), + "show-options", + "-g", + ) + flattened = tuple(str(token) for token in grouped.args) + + assert command_count(tuple(grouped.args)) == 2 + assert command_count(flattened) == 1 + + counts = CountingSink() + instrument(SubprocessEngine.for_server(server), counts).run(grouped) + assert counts.tmux_commands == 2 + + +def test_a_literal_semicolon_is_data_not_a_boundary() -> None: + """A ``";"`` a caller meant as text must not inflate the command count.""" + assert command_count(("send-keys", "-t", "%0", "echo hi ; echo bye")) == 1 + assert command_count(("send-keys", ";")) == 1 + assert command_count(("a", CommandSeparator(";"), "b")) == 2 + + +def test_error_hook_fires_and_the_error_still_propagates() -> None: + """A sink observes the failure; it does not swallow it.""" + seen: list[BaseException] = [] + + class _Recording: + def before_command(self, request: CommandRequest) -> None: + del request + + def after_command( + self, request: CommandRequest, result: CommandResult, state: t.Any + ) -> None: # pragma: no cover - the command raises + del request, result, state + + def handle_error( + self, request: CommandRequest, error: BaseException, state: t.Any + ) -> None: + del request, state + seen.append(error) + + sink = _Recording() + assert isinstance(sink, Sink) + engine = InstrumentedEngine(_ExplodingEngine(), sink) + + with pytest.raises(RuntimeError, match="boom: kill-server"): + engine.run(CommandRequest.from_args("kill-server")) + + assert len(seen) == 1 + assert isinstance(seen[0], RuntimeError) + + +def test_a_failed_command_is_still_charged_time() -> None: + """Duration accrues whether the command succeeded or raised.""" + counts = CountingSink() + engine = InstrumentedEngine(_ExplodingEngine(), counts) + + with pytest.raises(RuntimeError): + engine.run(CommandRequest.from_args("kill-server")) + + assert counts.requests == 1 + assert counts.elapsed_ns > 0 + + +def test_sinks_are_notified_in_the_order_given(server: Server) -> None: + """Ordering is part of the contract; an exporter may depend on it.""" + order: list[str] = [] + + class _Named: + def __init__(self, name: str) -> None: + self.name = name + + def before_command(self, request: CommandRequest) -> None: + del request + order.append(self.name) + + def after_command( + self, request: CommandRequest, result: CommandResult, state: t.Any + ) -> None: + del request, result, state + + def handle_error( + self, request: CommandRequest, error: BaseException, state: t.Any + ) -> None: # pragma: no cover - the command succeeds + del request, error, state + + engine = instrument( + SubprocessEngine.for_server(server), _Named("first"), _Named("second") + ) + engine.run(CommandRequest.from_args("show-options", "-g")) + + assert order == ["first", "second"] + + +def test_the_wrapper_forwards_what_the_protocol_does_not_cover( + server: Server, +) -> None: + """An engine's own surface stays reachable through the wrapper.""" + inner = SubprocessEngine.for_server(server) + engine = instrument(inner, CountingSink()) + + assert engine.inner is inner + assert engine.connection == inner.connection + + +def test_an_unwrapped_program_constructs_nothing(server: Server) -> None: + """The zero-overhead claim, stated as a test rather than as prose. + + Nothing in the seam creates a sink or a wrapper on its own, so a caller + that never asks for instrumentation runs the engine it built. + """ + engine = SubprocessEngine.for_server(server) + + assert not isinstance(engine, InstrumentedEngine) + assert type(engine).run is SubprocessEngine.run From 1f2ae6c0eb38bed707fc1251c97d8ce64a261288 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sat, 22 Aug 2026 19:56:47 -0500 Subject: [PATCH 07/16] Docs(api[engines]): Document the observation seam why: The module ships in libtmux.engines but the API page enumerated only base, connection, and subprocess, so autodoc rendered no page for it and a reader following the engines docs would not learn observation exists. what: - Give libtmux.engines.instrumentation its own section and automodule entry - Record the deliverable in CHANGES, including why observation is composed rather than installed and what command_count distinguishes --- CHANGES | 20 ++++++++++++++++++++ docs/api/libtmux.engines.md | 11 +++++++++++ 2 files changed, 31 insertions(+) diff --git a/CHANGES b/CHANGES index e00ebb59d5..5f52b3e3b5 100644 --- a/CHANGES +++ b/CHANGES @@ -90,6 +90,26 @@ dispatch to the ambient tmux server. Engines that name a server keep it. A custom `tmux_bin` selects a program rather than a server, so an engine carrying only one adopts the server's flags and keeps its own binary. +#### Observing what an engine runs (#739) + +{class}`~libtmux.engines.instrumentation.InstrumentedEngine` wraps any engine +and implements the same protocol, so counting or tracing tmux traffic no longer +means patching {mod}`subprocess` from outside — an approach that under-reports +an engine which never forks. Because observation is composed rather than +installed, a program that does not ask for it constructs nothing and runs the +code it ran before. + +{class}`~libtmux.engines.instrumentation.Sink` is the observer surface: a +before hook, an after hook, and an error hook, matching what OpenTelemetry and +Sentry already attach to on SQLAlchemy, so an exporter written for one reads +naturally here. {class}`~libtmux.engines.instrumentation.CountingSink` ships as +the worked example, reporting requests, tmux commands, and the commands that +rode inside another request's argv. + +{func}`~libtmux.engines.base.command_count` exposes that last distinction on its +own: a command group is one dispatch carrying several tmux commands, and a +literal `";"` a caller meant as data is not a boundary. + {class}`~libtmux.engines.connection.ServerConnection` is now the single place the tmux binary and the `-L`/`-S`/`-f`/`-2`/`-8` flags are computed; three separate copies previously disagreed about which flags to emit. It is derived diff --git a/docs/api/libtmux.engines.md b/docs/api/libtmux.engines.md index f555001585..862ca28c0f 100644 --- a/docs/api/libtmux.engines.md +++ b/docs/api/libtmux.engines.md @@ -55,3 +55,14 @@ single place either is computed. .. automodule:: libtmux.engines.subprocess :members: ``` + +## Observing an engine + +{class}`~libtmux.engines.instrumentation.InstrumentedEngine` wraps an engine and +satisfies the same protocol, so observation is a substitution rather than a +feature the engine carries. A program that wraps nothing constructs nothing. + +```{eval-rst} +.. automodule:: libtmux.engines.instrumentation + :members: +``` From 312a1f2ae8b4015ed34dd36808f66a8087f30d28 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 23 Aug 2026 08:29:10 -0500 Subject: [PATCH 08/16] Bench(feat[seam]): Share the benchmark primitives why: Isolating a tmux server, naming a shape, and reducing samples to percentiles are the same job whichever transport is being measured, and none of it needs an engine -- only the classic Server the library already ships. Keeping one copy per benchmark invites them to drift, and the isolation half is exactly where drift costs: it encodes two tmux behaviours that were each found the hard way. new_server pins a keepalive session because killing a cell's session otherwise drops the server to zero, and under tmux's exit-empty default the next build can reach the socket mid-shutdown. reap_stale_scratch leaves a directory alone while a tmux still answers on it, because stealing a concurrent run's servers would be worse than leaking one directory. what: - Add scripts/bench/primitives.py with server isolation, shape parsing, the classic build, and the percentile summary, alongside the reasoning for the two behaviours above - Return the reaped count rather than printing it, so the module needs no console and its caller keeps its own reporting - Cover the shape, the statistics, the keepalive, and the reaper's refusal to touch a live directory --- scripts/bench/primitives.py | 185 +++++++++++++++++++++++++++++++++ tests/test_bench_primitives.py | 131 +++++++++++++++++++++++ 2 files changed, 316 insertions(+) create mode 100644 scripts/bench/primitives.py create mode 100644 tests/test_bench_primitives.py diff --git a/scripts/bench/primitives.py b/scripts/bench/primitives.py new file mode 100644 index 0000000000..2cdf0d4958 --- /dev/null +++ b/scripts/bench/primitives.py @@ -0,0 +1,185 @@ +"""Isolation, shapes, and statistics shared by libtmux's benchmarks. + +Everything here works against what libtmux ships today -- the classic +:class:`~libtmux.Server` object hierarchy -- so a benchmark of any later +transport can reuse it without dragging that transport's imports along. + +The hermetic-isolation half is the part worth sharing rather than copying: it +encodes two tmux behaviours that cost real debugging time, and a second copy +would be free to forget either of them. See :func:`new_server` for the +``exit-empty`` race and :func:`reap_stale_scratch` for why a scratch directory +with a live server is left alone. +""" + +from __future__ import annotations + +import atexit +import contextlib +import itertools +import math +import os +import pathlib +import shutil +import statistics +import subprocess +import tempfile +import uuid + +from libtmux.server import Server + +__all__ = [ + "KEEPALIVE", + "SERVERS", + "SOCK_DIR", + "STAT_LABELS", + "build_classic", + "cleanup", + "new_server", + "parse_shape", + "percentile", + "reap_stale_scratch", + "summarize", + "uniq", +] + +STAT_LABELS = ("n", "min", "avg", "median", "p90", "p95", "p99", "max") + +_ctr = itertools.count() + +#: Short scratch root: an AF_UNIX path is capped at 107 bytes, and a socket +#: under a deep temporary directory is how that limit gets hit. +SOCK_DIR = pathlib.Path(tempfile.mkdtemp(prefix="ltbench-")) +SERVERS: list[Server] = [] +#: A session every bench server keeps for its whole life, so killing a cell's +#: session never drops the server to zero and trips tmux's exit-empty teardown. +KEEPALIVE = "keepalive" + + +def new_server() -> Server: + """Return a fresh isolated server on a unique socket under the scratch dir. + + The server is pinned alive by a keepalive session. Every cell kills its + session between builds, which would otherwise drop the server to zero + sessions; under tmux's ``exit-empty`` default the server then starts + exiting, and the next build's ``new-session`` can reach the still-bound + socket mid-shutdown and fail with "server exited unexpectedly". The race is + load-dependent, so it surfaced as an intermittent create failure rather than + an obvious teardown bug. Control mode never hit it -- its ``tmux -C`` + phantom session already pinned the server -- which is exactly why only the + subprocess cells were affected. + """ + srv = Server(socket_path=str(SOCK_DIR / f"{uuid.uuid4().hex[:8]}.sock")) + SERVERS.append(srv) + # The keepalive has to come first: `start-server` alone leaves a server with + # zero sessions, which exits immediately under the default, so there is no + # server left to set the option on. Creating a session that is never killed + # is what actually holds the floor above zero. + srv.cmd("new-session", "-d", "-s", KEEPALIVE) + srv.cmd("set-option", "-s", "exit-empty", "off") + return srv + + +def cleanup() -> None: + """Kill every server this process started and remove its scratch dir.""" + for srv in SERVERS: + with contextlib.suppress(Exception): + srv.kill() + # Backstop: SIGKILL any tmux server still bound to a socket in our dir. + with contextlib.suppress(Exception): + out = subprocess.run( + ["pgrep", "-f", f"tmux .*-S{SOCK_DIR}/"], + capture_output=True, + text=True, + check=False, + ).stdout.split() + for pid in out: + with contextlib.suppress(Exception): + os.kill(int(pid), 9) + with contextlib.suppress(Exception): + shutil.rmtree(SOCK_DIR, ignore_errors=True) + + +def reap_stale_scratch() -> int: + """Remove scratch dirs left by runs that died before their cleanup. + + :func:`cleanup` only knows *this* process's socket dir, so a run killed + before its ``atexit`` hook leaves its dir -- and any tmux still bound to + it -- behind for good. Those survivors keep consuming CPU and file + descriptors, and machine load is precisely what makes the server-teardown + race fire, so an unreaped leak feeds the very failure it came from. + + A dir with a live tmux is left alone: it may belong to a concurrent run, and + stealing another run's servers would be worse than leaking. That means hung + clients are only reclaimed once their server is gone, which is the + conservative trade. + + Returns + ------- + int + How many stale directories were removed. Reporting is the caller's + job, so this module needs no console of its own. + """ + reaped = 0 + for path in pathlib.Path(tempfile.gettempdir()).glob("ltbench-*"): + if path == SOCK_DIR or not path.is_dir(): + continue + with contextlib.suppress(Exception): + alive = subprocess.run( + ["pgrep", "-f", f"tmux .*-S{path}/"], + capture_output=True, + text=True, + check=False, + ).stdout.split() + if alive: + continue + shutil.rmtree(path, ignore_errors=True) + reaped += 1 + return reaped + + +atexit.register(cleanup) + + +def uniq() -> str: + """Return a process-unique session name (never collides across builds).""" + return f"b{next(_ctr)}" + + +def parse_shape(s: str) -> tuple[int, int]: + """'8x4' -> (8 windows, 4 panes-per-window).""" + w, _, p = s.lower().partition("x") + return int(w), int(p) + + +def build_classic(server: Server, name: str, wins: int, panes: int) -> None: + """Build the structure with the classic Server/Session/Window/Pane API.""" + session = server.new_session(session_name=name, window_name="w0") + for _ in range(panes - 1): + session.active_window.split() + for wi in range(1, wins): + window = session.new_window(window_name=f"w{wi}") + for _ in range(panes - 1): + window.split() + + +def percentile(sorted_vals: list[float], pct: float) -> float: + """Nearest-rank percentile of a pre-sorted sequence.""" + if not sorted_vals: + return float("nan") + rank = max(1, math.ceil(pct / 100.0 * len(sorted_vals))) + return sorted_vals[min(rank, len(sorted_vals)) - 1] + + +def summarize(samples: list[float]) -> dict[str, float]: + """Return min/avg/median/p90/p95/p99/max (and n) for *samples*.""" + s = sorted(samples) + return { + "n": float(len(s)), + "min": s[0], + "avg": statistics.fmean(s), + "median": statistics.median(s), + "p90": percentile(s, 90), + "p95": percentile(s, 95), + "p99": percentile(s, 99), + "max": s[-1], + } diff --git a/tests/test_bench_primitives.py b/tests/test_bench_primitives.py new file mode 100644 index 0000000000..994a4b6ca4 --- /dev/null +++ b/tests/test_bench_primitives.py @@ -0,0 +1,131 @@ +"""Tests for the benchmark primitives shared across libtmux's benchmarks.""" + +from __future__ import annotations + +import importlib.util +import math +import pathlib +import typing as t + +import pytest + +if t.TYPE_CHECKING: + import types + +_BENCH = pathlib.Path(__file__).parent.parent / "scripts" / "bench" + + +@pytest.fixture(scope="module") +def primitives() -> types.ModuleType: + """Load the primitives by path; ``scripts`` is not a package.""" + spec = importlib.util.spec_from_file_location( + "primitives", _BENCH / "primitives.py" + ) + assert spec is not None + assert spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def test_the_primitives_do_not_reach_the_experimental_package( + primitives: types.ModuleType, +) -> None: + """They exist to be shared by benchmarks that predate the engines. + + A primitive that imported an engine would make the seam's own benchmark + depend on the layer it is meant to be the baseline for. + """ + source = (_BENCH / "primitives.py").read_text(encoding="utf-8") + + assert "libtmux.experimental" not in source + assert "from libtmux.server import Server" in source + + +def test_parse_shape_reads_windows_by_panes(primitives: types.ModuleType) -> None: + """``8x4`` is eight windows of four panes, case-insensitively.""" + assert primitives.parse_shape("8x4") == (8, 4) + assert primitives.parse_shape("2X1") == (2, 1) + + +def test_uniq_never_repeats(primitives: types.ModuleType) -> None: + """Session names must not collide across builds in one process.""" + names = {primitives.uniq() for _ in range(50)} + + assert len(names) == 50 + + +def test_percentile_is_nearest_rank(primitives: types.ModuleType) -> None: + """The reported percentile is an observed sample, not an interpolation.""" + values = [1.0, 2.0, 3.0, 4.0] + + assert primitives.percentile(values, 100) == 4.0 + assert primitives.percentile(values, 50) in values + assert math.isnan(primitives.percentile([], 50)) + + +def test_summarize_reports_every_labelled_statistic( + primitives: types.ModuleType, +) -> None: + """Whatever STAT_LABELS advertises, summarize must actually return.""" + summary = primitives.summarize([1.0, 2.0, 3.0, 4.0]) + + assert set(summary) == set(primitives.STAT_LABELS) + assert summary["n"] == 4.0 + assert summary["min"] == 1.0 + assert summary["max"] == 4.0 + assert summary["median"] == 2.5 + + +def test_new_server_pins_itself_above_zero_sessions( + primitives: types.ModuleType, +) -> None: + """The keepalive is what stops tmux's exit-empty teardown racing a build. + + Without it a cell that kills its own session drops the server to zero, and + the next build can reach the socket mid-shutdown. Asserting the session + exists is asserting that guard is still in place. + """ + server = primitives.new_server() + try: + names = [s.name for s in server.sessions] + + assert primitives.KEEPALIVE in names + assert server.is_alive() + finally: + server.kill() + + +def test_build_classic_creates_the_requested_shape( + primitives: types.ModuleType, +) -> None: + """Two windows of two panes is what ``2x2`` has to produce.""" + server = primitives.new_server() + try: + primitives.build_classic(server, "shape", 2, 2) + session = next(s for s in server.sessions if s.name == "shape") + + assert len(session.windows) == 2 + for window in session.windows: + assert len(window.panes) == 2 + finally: + server.kill() + + +def test_reap_leaves_a_live_scratch_dir_alone( + primitives: types.ModuleType, +) -> None: + """A directory with a running server may belong to a concurrent run. + + Stealing another run's servers would be worse than leaking, so the reaper + is deliberately conservative and this pins that choice. + """ + server = primitives.new_server() + try: + assert primitives.SOCK_DIR.is_dir() + reaped = primitives.reap_stale_scratch() + + assert isinstance(reaped, int) + assert primitives.SOCK_DIR.is_dir() + finally: + server.kill() From eed72de7ff0d48e7eeed10456bf2ee3266336539 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 23 Aug 2026 08:29:27 -0500 Subject: [PATCH 09/16] Bench(feat[seam]): Measure what the current API costs why: The seam's claim is that the default path is unchanged and that observation costs nothing when nobody asks for it. Both are asserted in prose and neither has a number behind it. A baseline is also what any later transport has to be compared against, and there is currently nothing below the engines that reports one. what: - Add scripts/bench/current_api.py, timing topology construction, the classic hierarchy read, and per-request dispatch through the seam - Count requests separately from the tmux commands they carry, so a command group reads as two commands in one dispatch rather than as a saving; a change that merely moves cost between them stays visible as movement - Report the sessions a shape built apart from the sessions the server holds, since the isolation keepalive is one of the latter and not one of the former - Drive it through the shared primitives, so this benchmark and the engine benchmark above it measure with one ruler --- scripts/bench/current_api.py | 199 ++++++++++++++++++++++++++++++++ tests/test_bench_current_api.py | 107 +++++++++++++++++ 2 files changed, 306 insertions(+) create mode 100755 scripts/bench/current_api.py create mode 100644 tests/test_bench_current_api.py diff --git a/scripts/bench/current_api.py b/scripts/bench/current_api.py new file mode 100755 index 0000000000..a569142e38 --- /dev/null +++ b/scripts/bench/current_api.py @@ -0,0 +1,199 @@ +#!/usr/bin/env -S uv run --script +# /// script +# requires-python = ">=3.10" +# dependencies = ["libtmux"] +# /// +"""Measure what libtmux's current API costs against a live tmux server. + +This is the baseline, not a comparison. It exercises only what ships today -- +the classic :class:`~libtmux.Server` object hierarchy and the command execution +seam -- so its numbers are the reference any later transport has to beat. + +Three quantities, deliberately kept apart: + +- **construction**: wall time to build the requested topology. +- **enumeration**: wall time for ``server.sessions`` / ``.windows`` / ``.panes``, + the classic hierarchy read, with the row counts it returned. +- **dispatch**: wall time per :class:`~libtmux.engines.base.CommandRequest` + through :class:`~libtmux.engines.subprocess.SubprocessEngine`, alongside the + tmux commands those requests carried. + +The last pair is the point. A request and a tmux command are not the same +thing: a command group rides several commands inside one dispatch, and +:class:`~libtmux.engines.instrumentation.CountingSink` reports the difference +as ``inlined``. Measuring them separately is what keeps a later change that +merely moves work between them from reading as a win. + +Isolation, shape parsing, and statistics come from :mod:`primitives`, so +this benchmark and the engine benchmark above it measure with one ruler. +""" + +from __future__ import annotations + +import argparse +import contextlib +import json +import pathlib +import sys +import time +import typing as t + +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent)) + +from primitives import ( + build_classic, + new_server, + parse_shape, + summarize, + uniq, +) + +from libtmux.engines import ( + CommandRequest, + CommandSeparator, + CountingSink, + SubprocessEngine, + instrument, +) + +if t.TYPE_CHECKING: + from libtmux.server import Server + + +def _ms(samples_ns: list[int]) -> dict[str, float]: + """Summarise nanosecond samples as milliseconds, using the shared ruler.""" + return { + key: round(value, 4) + for key, value in summarize([value / 1e6 for value in samples_ns]).items() + } + + +def build_topology(server: Server, *, sessions: int, shape: str) -> dict[str, int]: + """Build *sessions* sessions of *shape*, timing the whole construction. + + Each session is built by :func:`primitives.build_classic`, the same + routine the engine benchmark uses for its ``classic`` lane, so a number + here and a number there describe the same work. + """ + wins, panes = parse_shape(shape) + started_ns = time.perf_counter_ns() + for _ in range(sessions): + build_classic(server, uniq(), wins, panes) + construction_ns = time.perf_counter_ns() - started_ns + # `sessions` counts what the server holds, which includes the keepalive + # session new_server() never kills; `built` counts what this run created. + # Reporting only the first would overstate the topology a shape asked for. + return { + "construction_ns": construction_ns, + "built": sessions, + "sessions": len(server.sessions), + "windows": sum(len(s.windows) for s in server.sessions), + } + + +def enumerate_classic(server: Server, *, rounds: int) -> dict[str, t.Any]: + """Time the classic hierarchy read, the API that ships today.""" + per_level: dict[str, list[int]] = {"sessions": [], "windows": [], "panes": []} + counts: dict[str, int] = {} + for _ in range(rounds): + for level in ("sessions", "windows", "panes"): + started_ns = time.perf_counter_ns() + rows = list(getattr(server, level)) + per_level[level].append(time.perf_counter_ns() - started_ns) + counts[level] = len(rows) + return { + "rows": counts, + "timings": {level: _ms(v) for level, v in per_level.items()}, + } + + +def dispatch_through_seam(server: Server, *, rounds: int) -> dict[str, t.Any]: + """Time requests through the seam, counting requests against tmux commands. + + The grouped request is the one that matters: it carries two tmux commands + in a single dispatch, so ``requests`` and ``tmux_commands`` diverge and + ``inlined`` reports by how much. + """ + counts = CountingSink() + engine = instrument(SubprocessEngine.for_server(server), counts) + + plain = CommandRequest.from_args("list-panes", "-a") + grouped = CommandRequest.from_args( + "set-option", + "-g", + "@bench", + "1", + CommandSeparator(";"), + "show-options", + "-g", + ) + + plain_ns: list[int] = [] + grouped_ns: list[int] = [] + for _ in range(rounds): + started_ns = time.perf_counter_ns() + engine.run(plain) + plain_ns.append(time.perf_counter_ns() - started_ns) + + started_ns = time.perf_counter_ns() + engine.run(grouped) + grouped_ns.append(time.perf_counter_ns() - started_ns) + + return { + "plain": _ms(plain_ns), + "grouped": _ms(grouped_ns), + "observed": { + "requests": counts.requests, + "tmux_commands": counts.tmux_commands, + "inlined": counts.inlined, + "elapsed_ms": round(counts.elapsed_ns / 1e6, 3), + }, + } + + +def run(*, sessions: int, shape: str, rounds: int) -> dict[str, t.Any]: + """Measure one shape end to end and return the machine-readable result.""" + server = new_server() + try: + topology = build_topology(server, sessions=sessions, shape=shape) + return { + "shape": {"sessions": sessions, "shape": shape, "rounds": rounds}, + "topology": { + "built": topology["built"], + "sessions": topology["sessions"], + "windows": topology["windows"], + "construction_ms": round(topology["construction_ns"] / 1e6, 3), + }, + "enumeration": enumerate_classic(server, rounds=rounds), + "dispatch": dispatch_through_seam(server, rounds=rounds), + } + finally: + with contextlib.suppress(Exception): + server.kill() + + +def main() -> int: + """Parse arguments, measure, and print the result as JSON.""" + parser = argparse.ArgumentParser(description="Benchmark libtmux's current API.") + parser.add_argument("--sessions", type=int, default=2) + parser.add_argument("--shape", default="2x1", help="windows x panes, e.g. 8x4") + parser.add_argument("--rounds", type=int, default=10) + args = parser.parse_args() + + if min(args.sessions, args.rounds) < 1: + parser.error("--sessions and --rounds must both be at least 1") + wins, panes = parse_shape(args.shape) + if min(wins, panes) < 1: + parser.error("--shape must name at least one window and one pane") + + print( + json.dumps( + run(sessions=args.sessions, shape=args.shape, rounds=args.rounds), + indent=2, + ) + ) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_bench_current_api.py b/tests/test_bench_current_api.py new file mode 100644 index 0000000000..5004d91572 --- /dev/null +++ b/tests/test_bench_current_api.py @@ -0,0 +1,107 @@ +"""Tests for the current-API benchmark at the command execution seam.""" + +from __future__ import annotations + +import importlib.util +import pathlib +import typing as t + +import pytest + +if t.TYPE_CHECKING: + import types + + from libtmux.server import Server + +_BENCH = pathlib.Path(__file__).parent.parent / "scripts" / "bench" + + +@pytest.fixture(scope="module") +def current_api() -> types.ModuleType: + """Load the benchmark by path; ``scripts`` is not a package.""" + spec = importlib.util.spec_from_file_location( + "current_api", _BENCH / "current_api.py" + ) + assert spec is not None + assert spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def test_the_benchmark_reaches_tmux_without_the_experimental_package( + current_api: types.ModuleType, +) -> None: + """The baseline must measure what ships, not what is being proposed. + + A benchmark that quietly imported the experimental engines would report a + number for the current API that the current API cannot produce. + """ + source = (_BENCH / "current_api.py").read_text(encoding="utf-8") + + assert "libtmux.experimental" not in source + assert "from libtmux.engines import" in source + assert "from libtmux.server import Server" in source + + +def test_the_baseline_uses_the_shared_ruler(current_api: types.ModuleType) -> None: + """Statistics come from bench_primitives, not a second implementation. + + Two benchmarks quoting different percentiles for the same samples would be + worse than having no baseline at all. + """ + summarised = current_api._ms([1_000_000, 2_000_000, 3_000_000]) + + assert summarised["n"] == 3.0 + assert summarised["min"] == 1.0 + assert summarised["max"] == 3.0 + assert "p95" in summarised + + +def test_enumeration_counts_the_live_hierarchy( + current_api: types.ModuleType, server: Server +) -> None: + """The classic read returns the rows the topology actually has.""" + built = current_api.build_topology(server, sessions=2, shape="2x1") + assert built["sessions"] == 2 + assert built["windows"] == 4 + + measured = current_api.enumerate_classic(server, rounds=2) + + assert measured["rows"]["sessions"] == 2 + assert measured["rows"]["windows"] == 4 + assert measured["timings"]["panes"]["n"] == 2.0 + assert measured["timings"]["sessions"]["median"] > 0 + + +def test_dispatch_separates_requests_from_the_commands_they_carry( + current_api: types.ModuleType, server: Server +) -> None: + """A command group is one dispatch carrying two tmux commands. + + This is the distinction the benchmark exists to keep visible: a change that + moves work from requests into inlining is movement, not a saving, and the + two counters are what make that legible. + """ + current_api.build_topology(server, sessions=1, shape="1x1") + + measured = current_api.dispatch_through_seam(server, rounds=3) + observed = measured["observed"] + + # three rounds, each running one plain request and one two-command group + assert observed["requests"] == 6 + assert observed["tmux_commands"] == 9 + assert observed["inlined"] == 3 + assert observed["elapsed_ms"] > 0 + + +def test_a_shape_smaller_than_one_is_refused( + current_api: types.ModuleType, monkeypatch: pytest.MonkeyPatch +) -> None: + """An empty shape measures nothing, so it is rejected rather than run.""" + monkeypatch.setattr("sys.argv", ["current_api.py", "--sessions", "0"]) + + with pytest.raises(SystemExit) as excinfo: + current_api.main() + + assert excinfo.value.code == 2 From 8abf41d12ab1bc264c2485f637c0bbe84ca9ffff Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Fri, 21 Aug 2026 12:48:05 -0500 Subject: [PATCH 10/16] Test(fix[server]): Stop a test depending on the caller's environment size why: test_new_session_shell_env forwarded dict(os.environ) to new-session, which becomes one -e KEY=VAL argument per variable. tmux sums every argument and refuses past MAX_IMSGSIZE, so the test's outcome was a function of whoever ran it: green on a lean CI runner, "command too long" under a rich interactive shell, where the environment alone can exceed 16 KiB before libtmux appends its format string. The test asserts nothing about the environment's contents, so the size was incidental. what: - Pass a small explicit environment, so the assertion is about what the test claims: window_command survives alongside environment= - Cover the ceiling deliberately instead, with a payload built to exceed it and an assertion that the refusal is loud and creates no session --- tests/test_server.py | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/tests/test_server.py b/tests/test_server.py index 6175a7f9ad..f05589a973 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -4,7 +4,6 @@ import functools import logging -import os import pathlib import shutil import subprocess @@ -207,9 +206,18 @@ def test_new_session_shell(server: Server) -> None: def test_new_session_shell_env(server: Server) -> None: - """Verify ``Server.new_session`` creates valid session running w/ command (#553).""" + """Verify ``Server.new_session`` creates valid session running w/ command (#553). + + The environment is a small explicit mapping rather than the caller's own. + Every variable becomes its own ``-e KEY=VAL`` argument, and tmux refuses a + command whose arguments exceed its message ceiling, so forwarding + ``os.environ`` made the outcome a function of whoever ran the suite: green + on a lean CI runner, ``command too long`` under a rich interactive shell. + :func:`test_new_session_rejects_an_environment_past_tmux_limit` covers that + ceiling deliberately. + """ cmd = "sleep 1m" - env = dict(os.environ) + env = {"LIBTMUX_TEST_ENV": "test_value", "LIBTMUX_TEST_OTHER": "second"} mysession = server.new_session( "test_new_session_env", window_command=cmd, @@ -227,6 +235,23 @@ def test_new_session_shell_env(server: Server) -> None: assert pane_start_command.replace('"', "") == cmd +def test_new_session_rejects_an_environment_past_tmux_limit(server: Server) -> None: + """An oversized environment fails loudly instead of truncating. + + tmux's client sums ``strlen(argv[i]) + 1`` across every argument and + refuses above ``MAX_IMSGSIZE`` (16384), so a large enough ``environment`` + cannot reach the server at all. Asserting the refusal keeps that a + deliberate, named behaviour rather than something a caller discovers when + their own shell happens to be big enough to trip it. + """ + oversized = {f"LIBTMUX_BULK_{index:03d}": "x" * 512 for index in range(64)} + + with pytest.raises(exc.LibTmuxException, match="command too long"): + server.new_session("test_new_session_oversized_env", environment=oversized) + + assert not server.has_session("test_new_session_oversized_env") + + @pytest.mark.skipif(True, reason="tmux 3.2 returns wrong width - test needs rework") def test_new_session_width_height(server: Server) -> None: """Verify ``Server.new_session`` creates valid session running w/ dimensions.""" From 299dedb1f40bda69533ba0f7fa6153096fe141bd Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 23 Aug 2026 09:33:28 -0500 Subject: [PATCH 11/16] Tests(fix[coverage]): Measure the library, not the tooling beside it `--cov=./` measures every Python file in the tree, so the benchmark and operations scripts under `scripts/` counted toward the library's coverage figure. They are development tooling: their uncovered branches are argument parsing, teardown paths, and failure reporting that exist to be read, not to be exercised by the suite. Counting them made the reported number a statement about the tooling's exercise rate rather than the library's. The two patterns mirror the `tests/` entries directly above them, covering `scripts/*.py` and one level of package beneath it, which is as deep as the tree goes. Verified by running one suite under both configurations: `scripts/bench/primitives.py` contributes 70 statements without the omit and disappears with it, taking the measured total from 3039 to 2969. --- pyproject.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 21d658f2b3..5e08a4f845 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -155,6 +155,8 @@ parallel = true omit = [ "*/_compat.py", "docs/conf.py", + "scripts/*.py", + "scripts/*/*.py", "tests/test_*.py", "tests/*/test_*.py", ] From fbdf19f9e03fd348793c4dc976c1d50123368c94 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 23 Aug 2026 09:38:21 -0500 Subject: [PATCH 12/16] Bench(fix[seam]): Resolve libtmux from the checkout, not the index The benchmark declared `dependencies = ["libtmux"]` with no `[tool.uv.sources]` entry, so running it the way its shebang says to -- `uv run --script scripts/bench/current_api.py` -- installed the released libtmux into the ephemeral environment instead of this working tree. Since it imports `libtmux.engines`, which no release carries, the documented invocation ended at `ModuleNotFoundError` rather than at a wrong number. A script that did import cleanly would have been worse: it would have reported the index's timings as the working tree's. Its existing tests could not see this. They load the module through `spec_from_file_location` into the development environment, where `import libtmux` succeeds regardless of what the inline block says, so the block was never exercised by anything. `tests/test_script_metadata.py` exercises the block directly, across every script under `scripts/` rather than this one. It compares the pin by resolving it against the script's own directory, so a script that moves between directories fails unless its `..` count follows -- the failure mode a rename invites. A scan-found-nothing guard keeps the parametrized checks from reporting green on an empty set. Verified: the documented invocation now builds and reports; removing the pin and mis-spelling its depth each fail with the specific reason. --- scripts/bench/current_api.py | 3 ++ tests/test_script_metadata.py | 97 +++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100644 tests/test_script_metadata.py diff --git a/scripts/bench/current_api.py b/scripts/bench/current_api.py index a569142e38..ca8143b6b8 100755 --- a/scripts/bench/current_api.py +++ b/scripts/bench/current_api.py @@ -2,6 +2,9 @@ # /// script # requires-python = ">=3.10" # dependencies = ["libtmux"] +# +# [tool.uv.sources] +# libtmux = { path = "../..", editable = true } # /// """Measure what libtmux's current API costs against a live tmux server. diff --git a/tests/test_script_metadata.py b/tests/test_script_metadata.py new file mode 100644 index 0000000000..da6c93be7f --- /dev/null +++ b/tests/test_script_metadata.py @@ -0,0 +1,97 @@ +"""Tests for the inline metadata that makes a repository script runnable. + +Every script under ``scripts/`` carries a PEP 723 block and a ``uv run +--script`` shebang, so its dependencies resolve without a prepared +environment. That block is invisible to the rest of the suite: the tests for a +given script import it by path into the already-installed development +environment, which satisfies ``import libtmux`` no matter what the block says. +These tests exercise the block itself. +""" + +from __future__ import annotations + +import pathlib +import typing as t + +import pytest + +# ``tomllib`` is stdlib from 3.11 onward. The package still supports 3.10, and +# mypy type-checks against that floor, so this module skips there rather than +# growing a dependency to parse a handful of comment lines. +tomllib = pytest.importorskip("tomllib") + +_REPO_ROOT = pathlib.Path(__file__).parent.parent +_SCRIPTS = _REPO_ROOT / "scripts" + +_BLOCK_OPEN = "# /// script" +_BLOCK_CLOSE = "# ///" + + +def _inline_metadata(script: pathlib.Path) -> dict[str, t.Any] | None: + """Return *script*'s PEP 723 table, or ``None`` when it carries no block.""" + lines = script.read_text(encoding="utf-8").splitlines() + if _BLOCK_OPEN not in lines: + return None + body: list[str] = [] + for line in lines[lines.index(_BLOCK_OPEN) + 1 :]: + if line == _BLOCK_CLOSE: + parsed: dict[str, t.Any] = tomllib.loads("\n".join(body)) + return parsed + body.append(line.removeprefix("#").removeprefix(" ")) + msg = f"{script} opens a PEP 723 block it never closes" + raise AssertionError(msg) + + +def _scripts_with_inline_metadata() -> list[pathlib.Path]: + """Every script carrying a PEP 723 block, in a stable order.""" + if not _SCRIPTS.is_dir(): + return [] + return sorted(p for p in _SCRIPTS.rglob("*.py") if _inline_metadata(p) is not None) + + +_PEP723 = _scripts_with_inline_metadata() +_IDS = [str(p.relative_to(_REPO_ROOT)) for p in _PEP723] + + +def test_the_scan_finds_scripts_to_check() -> None: + """Guard the parametrized tests below against passing on an empty set. + + They are parametrized over a filesystem scan, so a rename that moved every + script out of ``scripts/`` would leave them collecting nothing and reporting + green. + """ + assert _PEP723, f"no PEP 723 scripts found under {_SCRIPTS}" + + +@pytest.mark.parametrize("script", _PEP723, ids=_IDS) +def test_a_script_depending_on_libtmux_pins_this_checkout( + script: pathlib.Path, +) -> None: + """Resolving libtmux from the index measures the wrong library. + + Without a ``tool.uv.sources`` entry, ``uv`` installs the released libtmux + into the script's ephemeral environment. A benchmark then reports numbers + for whatever is on the index rather than for the working tree, and any + script reaching for an unreleased module fails outright at import. + + The path is compared by resolution rather than by spelling, so moving a + script between directories fails here unless its ``..`` count follows. + """ + metadata = _inline_metadata(script) + assert metadata is not None + + dependencies = metadata.get("dependencies", []) + if not any(d == "libtmux" or d.startswith("libtmux") for d in dependencies): + pytest.skip("does not depend on libtmux") + + sources = metadata.get("tool", {}).get("uv", {}).get("sources", {}) + assert "libtmux" in sources, ( + f"{script.name} depends on libtmux without a [tool.uv.sources] entry, " + "so uv resolves it from the index instead of this checkout" + ) + + pinned = (script.parent / sources["libtmux"]["path"]).resolve() + assert pinned == _REPO_ROOT.resolve(), ( + f"{script.name} pins libtmux at {pinned}, not the repository root" + ) + assert sources["libtmux"].get("editable") is True From c25f953e8907f530a7afe52f3567a611da5f7f1c Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 23 Aug 2026 09:58:22 -0500 Subject: [PATCH 13/16] Bench(fix[seam]): Exclude the caller's tmux configuration from the benchmark `new_server` promised a "fresh isolated server" and gave it a unique socket, which isolates it from other servers and from nothing else. tmux reads `~/.tmux.conf` when a server starts, so every measurement quietly carried whatever the machine set -- a `history-limit`, a hook, a slow `default-shell` -- and two machines could disagree about libtmux's cost for reasons that had nothing to do with libtmux. Demonstrated with a `HOME` holding a config that sets `history-limit 4242`: before, the benchmark's server reported 4242; after, it does not. That is the test, and it fails without the fix. `config_file=os.devnull` is the same spelling the engine grid arrived at independently for its own copy of this function, so the two now differ only in whether the module globals they read carry a leading underscore. --- scripts/bench/primitives.py | 10 +++++++++- tests/test_bench_primitives.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/scripts/bench/primitives.py b/scripts/bench/primitives.py index 2cdf0d4958..fce222420d 100644 --- a/scripts/bench/primitives.py +++ b/scripts/bench/primitives.py @@ -67,8 +67,16 @@ def new_server() -> Server: an obvious teardown bug. Control mode never hit it -- its ``tmux -C`` phantom session already pinned the server -- which is exactly why only the subprocess cells were affected. + + The configuration is excluded too. A unique socket isolates this server + from other servers; it says nothing about ``~/.tmux.conf``, which tmux + reads at server start and which would otherwise fold the machine's + history limits, hooks, and shell choice into every measurement. """ - srv = Server(socket_path=str(SOCK_DIR / f"{uuid.uuid4().hex[:8]}.sock")) + srv = Server( + socket_path=str(SOCK_DIR / f"{uuid.uuid4().hex[:8]}.sock"), + config_file=os.devnull, + ) SERVERS.append(srv) # The keepalive has to come first: `start-server` alone leaves a server with # zero sessions, which exits immediately under the default, so there is no diff --git a/tests/test_bench_primitives.py b/tests/test_bench_primitives.py index 994a4b6ca4..1938396dd2 100644 --- a/tests/test_bench_primitives.py +++ b/tests/test_bench_primitives.py @@ -129,3 +129,32 @@ def test_reap_leaves_a_live_scratch_dir_alone( assert primitives.SOCK_DIR.is_dir() finally: server.kill() + + +def test_new_server_ignores_the_calling_user_s_tmux_config( + primitives: types.ModuleType, + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A benchmark that reads ``~/.tmux.conf`` measures that file, not libtmux. + + tmux loads the invoking user's configuration when a server starts, so + without an explicit one the numbers move with whatever the machine happens + to set -- history limits, hooks, a slow ``default-shell``. A unique socket + isolates the server from other servers; it does nothing about the + configuration, and this module promises isolation rather than a fresh + socket. + """ + (tmp_path / ".tmux.conf").write_text( + "set-option -g history-limit 4242\n", encoding="utf-8" + ) + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.delenv("XDG_CONFIG_HOME", raising=False) + + server = primitives.new_server() + try: + limit = server.cmd("show-options", "-gv", "history-limit").stdout + + assert limit != ["4242"] + finally: + server.kill() From 8f5ffb6257725f155eb38979d71d6d1247e636f2 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 23 Aug 2026 10:52:06 -0500 Subject: [PATCH 14/16] Bench(fix[seam]): Stop the reaper deleting a concurrent run's scratch directory `reap_stale_scratch` decided a scratch directory was abandoned by looking for a tmux running out of it. A directory exists from the moment a run imports this module, but its first server appears only at the first `new_server`, so during that window every live run looked abandoned and was deleted -- socket directory and all -- by any other run that happened to reap. Under `pytest -n auto` the workers import at different moments, so the window is always open somewhere. The failure surfaced far from its cause, as `new-session: error creating /tmp/ltbench-XXXX/YYYY.sock (No such file or directory)` in a worker whose directory had been removed underneath it, and as `assert 'keepalive' in []` where the list accessor's empty-on-error contract turned the missing socket into a silent empty list rather than a raised error. Liveness is now the owning process, recorded in `owner.pid` when the directory is created and checked with `os.kill(pid, 0)`. `PermissionError` counts as alive: another user's process is running, merely not ours to signal. A directory naming no owner -- one from before this file recorded them, or one caught between creation and its claim -- is judged by age instead, and only then does the tmux probe run. Every unknown resolves toward keeping the directory, so the failure mode stays a leak rather than a theft. The test that claimed to pin this only ever checked the reaper's own directory, which is skipped by identity, so it could not have failed for the reason it named; it is renamed to say what it actually pins. The four cases that needed a rule are covered directly, and removing the owner check turns exactly the two sparing tests red while the two reaping tests stay green. --- scripts/bench/primitives.py | 60 +++++++++++++++++-- tests/test_bench_primitives.py | 102 +++++++++++++++++++++++++++++++-- 2 files changed, 154 insertions(+), 8 deletions(-) diff --git a/scripts/bench/primitives.py b/scripts/bench/primitives.py index fce222420d..707f875e29 100644 --- a/scripts/bench/primitives.py +++ b/scripts/bench/primitives.py @@ -23,12 +23,14 @@ import statistics import subprocess import tempfile +import time import uuid from libtmux.server import Server __all__ = [ "KEEPALIVE", + "OWNER_PID", "SERVERS", "SOCK_DIR", "STAT_LABELS", @@ -46,9 +48,22 @@ _ctr = itertools.count() +#: Names the process that owns a scratch directory. A concurrent run is +#: identified by its pid rather than by whether a tmux happens to be running in +#: its directory: the directory exists from import, but no server does until the +#: first :func:`new_server`, and a reaper that only looks for tmux deletes other +#: runs during that window. +OWNER_PID = "owner.pid" + +#: How long a scratch directory carrying no owner file is left alone. It covers +#: the instant between creating a directory and claiming it, and directories +#: left by versions that predate the owner file. +_ADOPTION_GRACE_SECONDS = 300.0 + #: Short scratch root: an AF_UNIX path is capped at 107 bytes, and a socket #: under a deep temporary directory is how that limit gets hit. SOCK_DIR = pathlib.Path(tempfile.mkdtemp(prefix="ltbench-")) +(SOCK_DIR / OWNER_PID).write_text(f"{os.getpid()}\n", encoding="utf-8") SERVERS: list[Server] = [] #: A session every bench server keeps for its whole life, so killing a cell's #: session never drops the server to zero and trips tmux's exit-empty teardown. @@ -107,6 +122,28 @@ def cleanup() -> None: shutil.rmtree(SOCK_DIR, ignore_errors=True) +def _owner_is_alive(path: pathlib.Path) -> bool | None: + """Say whether *path*'s owning process still exists. + + Returns ``None`` when the directory names no owner, which is not the same + answer as "the owner is gone": a directory written by a version that predates + :data:`OWNER_PID`, or caught in the instant between being created and being + claimed, has an owner this cannot see. + """ + try: + pid = int((path / OWNER_PID).read_text(encoding="utf-8").strip()) + except (OSError, ValueError): + return None + try: + os.kill(pid, 0) + except PermissionError: + # Someone else's process: running, merely not ours to signal. + return True + except OSError: + return False + return True + + def reap_stale_scratch() -> int: """Remove scratch dirs left by runs that died before their cleanup. @@ -116,10 +153,18 @@ def reap_stale_scratch() -> int: descriptors, and machine load is precisely what makes the server-teardown race fire, so an unreaped leak feeds the very failure it came from. - A dir with a live tmux is left alone: it may belong to a concurrent run, and - stealing another run's servers would be worse than leaking. That means hung - clients are only reclaimed once their server is gone, which is the - conservative trade. + A directory belonging to a live run is left alone: stealing another run's + servers is worse than leaking. Liveness is decided by the owning process + named in :data:`OWNER_PID`, not by whether a tmux is running there -- a + directory exists from the moment its run imports this module, while its + first server appears only at the first :func:`new_server`. A reaper that + asked only about tmux deleted concurrent runs during that window, taking + the socket directory out from under them mid-benchmark. + + Where no owner is named -- a directory from before this file recorded one, + or one caught between creation and its claim -- the choice is made by age, + and only then does the tmux probe decide. Every unknown resolves toward + keeping the directory, so the failure mode stays a leak. Returns ------- @@ -132,6 +177,13 @@ def reap_stale_scratch() -> int: if path == SOCK_DIR or not path.is_dir(): continue with contextlib.suppress(Exception): + owner = _owner_is_alive(path) + if owner: + continue + if owner is None and ( + time.time() - path.stat().st_mtime < _ADOPTION_GRACE_SECONDS + ): + continue alive = subprocess.run( ["pgrep", "-f", f"tmux .*-S{path}/"], capture_output=True, diff --git a/tests/test_bench_primitives.py b/tests/test_bench_primitives.py index 1938396dd2..b4828f6601 100644 --- a/tests/test_bench_primitives.py +++ b/tests/test_bench_primitives.py @@ -4,7 +4,11 @@ import importlib.util import math +import os import pathlib +import subprocess +import sys +import time import typing as t import pytest @@ -112,13 +116,13 @@ def test_build_classic_creates_the_requested_shape( server.kill() -def test_reap_leaves_a_live_scratch_dir_alone( +def test_reap_never_removes_its_own_scratch_dir( primitives: types.ModuleType, ) -> None: - """A directory with a running server may belong to a concurrent run. + """The reaper must not delete the directory it is running out of. - Stealing another run's servers would be worse than leaking, so the reaper - is deliberately conservative and this pins that choice. + This pins only the own-directory case, which the reaper answers by identity. + The concurrent-run cases are below; they are the ones that needed a rule. """ server = primitives.new_server() try: @@ -158,3 +162,93 @@ def test_new_server_ignores_the_calling_user_s_tmux_config( assert limit != ["4242"] finally: server.kill() + + +def test_reap_spares_a_directory_a_live_run_owns( + primitives: types.ModuleType, + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A concurrent run owns its directory from import, before any server exists. + + ``SOCK_DIR`` is created when a run imports this module; its first server + arrives only at the first :func:`new_server`. Deciding liveness by looking + for a tmux therefore called every run "stale" during that window and deleted + it, which surfaced as ``new-session: error creating ... (No such file or + directory)`` in a run whose directory had been removed underneath it. + """ + monkeypatch.setattr(primitives.tempfile, "gettempdir", lambda: str(tmp_path)) + victim = tmp_path / "ltbench-concurrent" + victim.mkdir() + (victim / primitives.OWNER_PID).write_text(f"{os.getpid()}\n", encoding="utf-8") + + reaped = primitives.reap_stale_scratch() + + assert victim.is_dir() + assert reaped == 0 + + +def test_reap_removes_a_directory_whose_owner_is_gone( + primitives: types.ModuleType, + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Reaping still happens; sparing live runs must not disable it. + + The owner here is a process that has been waited on, so its pid names + nothing. Ageing the directory past the grace period keeps this about the + owner rather than about the clock. + """ + monkeypatch.setattr(primitives.tempfile, "gettempdir", lambda: str(tmp_path)) + dead = subprocess.Popen([sys.executable, "-c", ""]) + dead.wait() + stale = tmp_path / "ltbench-abandoned" + stale.mkdir() + (stale / primitives.OWNER_PID).write_text(f"{dead.pid}\n", encoding="utf-8") + + reaped = primitives.reap_stale_scratch() + + assert not stale.exists() + assert reaped == 1 + + +def test_reap_spares_a_young_directory_that_names_no_owner( + primitives: types.ModuleType, + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Between creating a directory and claiming it, no owner is readable. + + That instant looks exactly like a directory from a version that never wrote + an owner, so both are answered the same way: too young to judge, keep it. + """ + monkeypatch.setattr(primitives.tempfile, "gettempdir", lambda: str(tmp_path)) + unclaimed = tmp_path / "ltbench-unclaimed" + unclaimed.mkdir() + + reaped = primitives.reap_stale_scratch() + + assert unclaimed.is_dir() + assert reaped == 0 + + +def test_reap_removes_an_old_directory_that_names_no_owner( + primitives: types.ModuleType, + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The grace period is a delay, not an exemption. + + A directory left by an older version is still debris; it is simply given + long enough that a directory being created right now is not mistaken for it. + """ + monkeypatch.setattr(primitives.tempfile, "gettempdir", lambda: str(tmp_path)) + ancient = tmp_path / "ltbench-ancient" + ancient.mkdir() + old = time.time() - primitives._ADOPTION_GRACE_SECONDS - 60 + os.utime(ancient, (old, old)) + + reaped = primitives.reap_stale_scratch() + + assert not ancient.exists() + assert reaped == 1 From 301a2b8693654310fb083e38d30e09771d586743 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 23 Aug 2026 12:32:52 -0500 Subject: [PATCH 15/16] Tests(refactor): Mirror the scripts tree under tests/scripts `tests/` root is the library's namespace -- `tests/test_server.py` covers `src/libtmux/server.py`, `tests/experimental/engines/` covers `src/libtmux/experimental/engines/` -- and a top-level directory names the repository tree it tests, which is what `tests/docs/` already does for `docs/`. Tests for `scripts/` were sitting in the library's half of that namespace under a `test_bench_`/`test_script_` prefix, so the prefix was doing a directory's job and claiming the wrong tree while doing it. tests/test_bench_primitives.py -> tests/scripts/bench/test_primitives.py tests/test_bench_current_api.py -> tests/scripts/bench/test_current_api.py tests/test_script_metadata.py -> tests/scripts/test_metadata.py Each of these finds the repository root by counting directories up from its own file, so moving them deeper silently pointed every lookup at the wrong place -- the same counting mistake as a PEP 723 `path = ".."` that no longer reaches the root. The counts move with the files. --- tests/scripts/__init__.py | 1 + tests/scripts/bench/__init__.py | 1 + .../bench/test_current_api.py} | 2 +- .../bench/test_primitives.py} | 2 +- tests/{test_script_metadata.py => scripts/test_metadata.py} | 2 +- 5 files changed, 5 insertions(+), 3 deletions(-) create mode 100644 tests/scripts/__init__.py create mode 100644 tests/scripts/bench/__init__.py rename tests/{test_bench_current_api.py => scripts/bench/test_current_api.py} (98%) rename tests/{test_bench_primitives.py => scripts/bench/test_primitives.py} (99%) rename tests/{test_script_metadata.py => scripts/test_metadata.py} (98%) diff --git a/tests/scripts/__init__.py b/tests/scripts/__init__.py new file mode 100644 index 0000000000..ea48082376 --- /dev/null +++ b/tests/scripts/__init__.py @@ -0,0 +1 @@ +"""Tests for scripts/.""" diff --git a/tests/scripts/bench/__init__.py b/tests/scripts/bench/__init__.py new file mode 100644 index 0000000000..d20d1a6244 --- /dev/null +++ b/tests/scripts/bench/__init__.py @@ -0,0 +1 @@ +"""Tests for scripts/bench/.""" diff --git a/tests/test_bench_current_api.py b/tests/scripts/bench/test_current_api.py similarity index 98% rename from tests/test_bench_current_api.py rename to tests/scripts/bench/test_current_api.py index 5004d91572..43c9d703ac 100644 --- a/tests/test_bench_current_api.py +++ b/tests/scripts/bench/test_current_api.py @@ -13,7 +13,7 @@ from libtmux.server import Server -_BENCH = pathlib.Path(__file__).parent.parent / "scripts" / "bench" +_BENCH = pathlib.Path(__file__).parents[3] / "scripts" / "bench" @pytest.fixture(scope="module") diff --git a/tests/test_bench_primitives.py b/tests/scripts/bench/test_primitives.py similarity index 99% rename from tests/test_bench_primitives.py rename to tests/scripts/bench/test_primitives.py index b4828f6601..1eee88cab5 100644 --- a/tests/test_bench_primitives.py +++ b/tests/scripts/bench/test_primitives.py @@ -16,7 +16,7 @@ if t.TYPE_CHECKING: import types -_BENCH = pathlib.Path(__file__).parent.parent / "scripts" / "bench" +_BENCH = pathlib.Path(__file__).parents[3] / "scripts" / "bench" @pytest.fixture(scope="module") diff --git a/tests/test_script_metadata.py b/tests/scripts/test_metadata.py similarity index 98% rename from tests/test_script_metadata.py rename to tests/scripts/test_metadata.py index da6c93be7f..25f1b7b8f0 100644 --- a/tests/test_script_metadata.py +++ b/tests/scripts/test_metadata.py @@ -20,7 +20,7 @@ # growing a dependency to parse a handful of comment lines. tomllib = pytest.importorskip("tomllib") -_REPO_ROOT = pathlib.Path(__file__).parent.parent +_REPO_ROOT = pathlib.Path(__file__).parents[2] _SCRIPTS = _REPO_ROOT / "scripts" _BLOCK_OPEN = "# /// script" From 3de106212136e0a2035861f68ebd401e95b95e21 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 23 Aug 2026 12:44:10 -0500 Subject: [PATCH 16/16] Tests(fix[coverage]): Stop the omit list depending on directory depth `tests/*/test_*.py` reaches one directory below `tests/`, and `*` does not cross a separator, so every test file nested deeper was measured as though it were library code. Sixty-eight already were. Because those files execute, they report near-total coverage and lift the figure rather than lowering it: the suite reports 84% with them counted and 78% without, so 5,530 statements of test code were worth six points of the library's score. `**` matches any number of nested directories, so one pattern replaces the two that enumerated levels, and a new directory cannot silently fall outside it. The same collapse applies to the `scripts/` entries added alongside them, which had the same one-level ceiling. This is the third place in this tree where a path's depth was written down and then stopped matching when a file moved -- the others being a PEP 723 `path = ".."` and the `parents[1]` lookups in the tests themselves. Each failed silently rather than raising. The reported figure drops six points because it stops averaging the library with its own test suite. --- pyproject.toml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 5e08a4f845..23b5234b9e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -155,10 +155,8 @@ parallel = true omit = [ "*/_compat.py", "docs/conf.py", - "scripts/*.py", - "scripts/*/*.py", - "tests/test_*.py", - "tests/*/test_*.py", + "scripts/**/*.py", + "tests/**/test_*.py", ] [tool.coverage.report]