From 5e51c9161d5f92eef8ed1784ddb82a822348f321 Mon Sep 17 00:00:00 2001 From: pbean Date: Sun, 28 Jun 2026 13:13:13 -0700 Subject: [PATCH 1/9] refactor(adapters): make the tmux backend an extension point Phase A of the adapter-seam close-out: funnel every tmux invocation through one overridable spawn primitive so a tmux-family backend (a future native- Windows psmux) can subclass it without editing tmux_backend.py. - New adapters/tmux_base.py :: BaseTmuxBackend holds all argv construction and every contract method, routed through a single _run() primitive (_tmux uses check=True; the ~14 tolerant sites use check=False, keeping their existing which-guards and exception handling). TmuxError/TMUX_TIMEOUT_S/ PARKED_RETURN_DETACH move here. - tmux_backend.py slims to TmuxMultiplexer(BaseTmuxBackend) plus back-compat re-exports (subprocess/shutil/TmuxError/TMUX_TIMEOUT_S/PARKED_RETURN_DETACH). - Portability guard: TMUX_BACKEND string -> TMUX_BACKENDS set covering both files; the sole ["tmux", ...] literal now lives in tmux_base._run. - Retarget test monkeypatches tmux_backend.{subprocess,shutil} -> tmux_base (the spawn seam moved modules). Behavior-preserving on POSIX. Full suite green (1119 passed); trunk clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/automator/adapters/tmux_backend.py | 355 ++----------------------- src/automator/adapters/tmux_base.py | 298 +++++++++++++++++++++ tests/test_generic_tmux.py | 4 +- tests/test_multiplexer.py | 4 +- tests/test_portability_guard.py | 15 +- tests/test_runs.py | 8 +- tests/test_tui_data.py | 14 +- tests/test_tui_launch.py | 53 ++-- 8 files changed, 371 insertions(+), 380 deletions(-) create mode 100644 src/automator/adapters/tmux_base.py diff --git a/src/automator/adapters/tmux_backend.py b/src/automator/adapters/tmux_backend.py index 9d7a3cc..ed2bf5c 100644 --- a/src/automator/adapters/tmux_backend.py +++ b/src/automator/adapters/tmux_backend.py @@ -1,339 +1,30 @@ -"""tmux backend for the terminal-multiplexer seam. - -This is the **only** file in the codebase allowed to shell out to ``tmux`` — -every POSIX-shell trailer and tmux invocation is quarantined here so a future -non-POSIX backend (an eventual native-Windows "psmux") can replace it wholesale. -See :mod:`.multiplexer` for the contract. - -Phase 1 implemented the subset the generic adapter drives plus the parked-window -trailer; Phase 2 fills in the rest as the other call sites (``runs.py``, -``tui/launch.py``, ``probe.py``, ``tui/data.py``) migrate onto the seam. +"""POSIX tmux backend for the terminal-multiplexer seam. + +The tmux/POSIX-shell quarantine spans this file and its base +(:mod:`.tmux_base`) — together they are the **only** place in the codebase +allowed to shell out to ``tmux``, so a future non-POSIX backend (an eventual +native-Windows "psmux") can replace them wholesale. All argv construction and +the single spawn primitive live in :class:`~.tmux_base.BaseTmuxBackend`; this +leaf is the POSIX implementation and inherits the full contract unchanged. See +:mod:`.multiplexer` for the contract. + +``subprocess`` and ``shutil`` are imported (and re-exported) here so existing +callers and tests can still reach the spawn seam via ``tmux_backend.subprocess`` +/ ``tmux_backend.shutil``; the live calls run through ``tmux_base``. """ from __future__ import annotations -import os -import shlex -import shutil -import subprocess -import time -from pathlib import Path - -from .multiplexer import MultiplexerError, TerminalMultiplexer - -TMUX_TIMEOUT_S = 30 -# Per-window option value (vs a pane id) telling the parked trailer to detach the -# client rather than switch it. Pane ids are %N, so this never collides with one. -PARKED_RETURN_DETACH = "detach" - - -class TmuxError(MultiplexerError): - pass - - -class TmuxMultiplexer(TerminalMultiplexer): - def _tmux(self, *args: str) -> str: - proc = subprocess.run( - ["tmux", *args], capture_output=True, text=True, timeout=TMUX_TIMEOUT_S - ) - if proc.returncode != 0: - raise TmuxError(f"tmux {' '.join(args[:2])} failed: {proc.stderr.strip()}") - return proc.stdout.strip() - - # ----------------------------------------------------------- sessions - - def has_session(self, name: str) -> bool: - # has-session returns nonzero for an absent session (a normal answer, not an - # error), so this can't go through _tmux. But a timeout or a missing binary - # is a real backend failure: raise the seam type so callers catch it via - # MultiplexerError instead of a raw subprocess error escaping. - try: - probe = subprocess.run( - ["tmux", "has-session", "-t", f"={name}"], - capture_output=True, - timeout=TMUX_TIMEOUT_S, - ) - except (subprocess.TimeoutExpired, OSError) as exc: - raise TmuxError(f"tmux has-session failed: {exc}") from exc - return probe.returncode == 0 - - def new_session( - self, name: str, cwd: Path, cols: int | None = None, lines: int | None = None - ) -> None: - # Window 0 is a plain shell so the session survives task windows closing. - # Geometry is pinned only when both dimensions are given (detached agent - # sessions); the control session omits it and takes tmux's default size. - geometry = ["-x", str(cols), "-y", str(lines)] if cols and lines else [] - self._tmux("new-session", "-d", "-s", name, "-c", str(cwd), *geometry) - - def set_session_option(self, name: str, option: str, value: str) -> None: - # set-option has no '=' exact-match form; callers pass a unique full - # session name so plain-name targeting resolves it unambiguously. - self._tmux("set-option", "-t", name, option, value) - - def kill_session(self, name: str) -> None: - # Tolerant of tmux being absent / the session already gone: a best-effort - # teardown backstop, never a hard failure. - if not shutil.which("tmux"): - return - try: - subprocess.run( - ["tmux", "kill-session", "-t", f"={name}"], - capture_output=True, - timeout=TMUX_TIMEOUT_S, - ) - except (subprocess.SubprocessError, OSError): - pass - - def list_sessions(self) -> list[str]: - # [] when tmux is missing, no server is running, or the query fails — the - # absence of sessions and the absence of tmux are indistinguishable here - # and callers treat both as "nothing live". - if not shutil.which("tmux"): - return [] - try: - proc = subprocess.run( - ["tmux", "list-sessions", "-F", "#{session_name}"], - capture_output=True, - text=True, - timeout=TMUX_TIMEOUT_S, - ) - except (subprocess.SubprocessError, OSError): - return [] - if proc.returncode != 0: # no server / no sessions - return [] - return [line for line in proc.stdout.splitlines() if line] - - def session_options(self, option: str) -> dict[str, str]: - # Map session name -> value of ``option`` ("" when unset). Same missing - # tmux / no-server tolerance as list_sessions(). - if not shutil.which("tmux"): - return {} - try: - proc = subprocess.run( - ["tmux", "list-sessions", "-F", f"#{{session_name}}\t#{{{option}}}"], - capture_output=True, - text=True, - timeout=TMUX_TIMEOUT_S, - ) - except (subprocess.SubprocessError, OSError): - return {} - if proc.returncode != 0: # no server / no sessions - return {} - options: dict[str, str] = {} - for line in proc.stdout.splitlines(): - name, _, value = line.partition("\t") - if name: - options[name] = value - return options - - # ------------------------------------------------------------ windows - - def new_window( - self, session: str, name: str, cwd: Path, env: dict[str, str], command: str - ) -> str: - env_args: list[str] = [] - for key, value in env.items(): - env_args += ["-e", f"{key}={value}"] - return self._tmux( - "new-window", - "-t", - f"={session}:", - "-n", - name, - "-c", - str(cwd), - "-P", - "-F", - "#{window_id}", - *env_args, - command, - ) - - def new_parked_window( - self, session: str, name: str, cwd: Path, argv: list[str], return_opt: str - ) -> str: - # The window runs under explicit `sh -c` (the user's login shell may be - # fish); the trailing `read` keeps the exit status inspectable instead of - # tmux closing the window the moment the process exits. After the read the - # return trailer switches an attached client back to its origin pane: - # - return_opt == a pane id (%N): switch that client back there - # (`switch-client -l` is a best-effort fallback when it is gone); - # - return_opt == PARKED_RETURN_DETACH: detach the client so a blocking - # `tmux attach` returns and a suspended TUI resumes; - # - unset/empty: nobody attached interactively -> park as-is. - return_trailer = ( - f"ret=$(tmux show-options -wqv {return_opt} 2>/dev/null); " - f'if [ "$ret" = "{PARKED_RETURN_DETACH}" ]; then tmux detach-client 2>/dev/null; ' - 'elif [ -n "$ret" ]; then ' - 'tmux switch-client -t "$ret" 2>/dev/null || tmux switch-client -l 2>/dev/null; ' - "fi" - ) - inner = shlex.join(argv) - shell = ( - f'{inner}; ec=$?; echo "[bmad-auto exited $ec — press enter]"; ' - f"read -r; {return_trailer}" - ) - return self._tmux( - "new-window", - "-d", - "-P", - "-F", - "#{window_id}", - "-t", - f"={session}:", - "-n", - name, - "-c", - str(cwd), - "sh", - "-c", - shell, - ) - - def list_window_ids(self, session: str) -> list[str]: - # display-message -t exits 0 with empty output, so list the - # session's window ids and check membership instead. - probe = subprocess.run( - ["tmux", "list-windows", "-t", f"={session}", "-F", "#{window_id}"], - capture_output=True, - text=True, - timeout=TMUX_TIMEOUT_S, - ) - if probe.returncode != 0: - return [] - return probe.stdout.split() - - def pipe_pane(self, window_id: str, log_file: Path) -> None: - # A CLI that crashes on launch (bad args, instant auth failure) can take - # its window down before pipe-pane attaches, which races as "can't find - # window". That is not a setup failure, so tolerate it instead of raising. - try: - self._tmux("pipe-pane", "-t", window_id, "-o", f"cat >> {shlex.quote(str(log_file))}") - except TmuxError: - pass - - def send_text(self, window_id: str, text: str) -> None: - self._tmux("send-keys", "-t", window_id, "-l", text) - time.sleep(0.3) # let the TUI ingest the paste before submitting - self._tmux("send-keys", "-t", window_id, "Enter") - - def kill_window(self, target: str) -> None: - subprocess.run( - ["tmux", "kill-window", "-t", target], - capture_output=True, - timeout=TMUX_TIMEOUT_S, - ) - - def list_windows(self, session: str, fields: list[str]) -> list[tuple[str, ...]]: - fmt = "\t".join(f"#{{{field}}}" for field in fields) - probe = subprocess.run( - ["tmux", "list-windows", "-t", f"={session}", "-F", fmt], - capture_output=True, - text=True, - timeout=TMUX_TIMEOUT_S, - ) - if probe.returncode != 0: - return [] - rows: list[tuple[str, ...]] = [] - for line in probe.stdout.splitlines(): - parts = line.split("\t") - parts += [""] * (len(fields) - len(parts)) # tolerate unset trailing fields - rows.append(tuple(parts[: len(fields)])) - return rows - - def window_alive(self, session: str, window_id: str) -> bool: - return window_id in self.list_window_ids(session) - - def select_window(self, target: str) -> None: - subprocess.run( - ["tmux", "select-window", "-t", target], - capture_output=True, - timeout=TMUX_TIMEOUT_S, - ) - - def set_window_option(self, target: str, option: str, value: str) -> None: - subprocess.run( - ["tmux", "set-option", "-w", "-t", target, option, value], - capture_output=True, - timeout=TMUX_TIMEOUT_S, - ) - - def unset_window_option(self, target: str, option: str) -> None: - subprocess.run( - ["tmux", "set-option", "-wu", "-t", target, option], - capture_output=True, - timeout=TMUX_TIMEOUT_S, - ) - - def show_window_option(self, target: str, option: str) -> str: - proc = subprocess.run( - ["tmux", "show-options", "-wqv", "-t", target, option], - capture_output=True, - text=True, - timeout=TMUX_TIMEOUT_S, - ) - return proc.stdout.strip() if proc.returncode == 0 else "" - - # ----------------------------------------------------- client / attach - - def attach_target_argv(self, target: str) -> list[str]: - # Inside tmux, nesting an attach is refused, so switch this client - # instead (a `switch-client -l` brings it back). - if os.environ.get("TMUX"): - return ["tmux", "switch-client", "-t", target] - return ["tmux", "attach", "-t", target] - - def current_pane_id(self) -> str | None: - return self._display_message("#{pane_id}") - - def current_window_id(self) -> str | None: - return self._display_message("#{window_id}") - - def current_session(self) -> str | None: - return self._display_message("#{session_name}") - - def _display_message(self, fmt: str) -> str | None: - """Resolve a tmux format string against this process's client, or None - when not inside tmux / tmux is unavailable.""" - try: - proc = subprocess.run( - ["tmux", "display-message", "-p", fmt], - capture_output=True, - text=True, - timeout=TMUX_TIMEOUT_S, - ) - except (subprocess.SubprocessError, OSError): - return None - return proc.stdout.strip() if proc.returncode == 0 else None - - def detach_client(self) -> None: - subprocess.run(["tmux", "detach-client"], capture_output=True, timeout=TMUX_TIMEOUT_S) +import shutil # noqa: F401 — re-exported for callers/tests reaching the spawn seam +import subprocess # noqa: F401 — re-exported for callers/tests reaching the spawn seam - def switch_client(self, target: str, last_fallback: bool = False) -> bool: - proc = subprocess.run( - ["tmux", "switch-client", "-t", target], - capture_output=True, - timeout=TMUX_TIMEOUT_S, - ) - if proc.returncode == 0: - return True - if last_fallback: - fb = subprocess.run( - ["tmux", "switch-client", "-l"], - capture_output=True, - timeout=TMUX_TIMEOUT_S, - ) - return fb.returncode == 0 - return False +from .tmux_base import PARKED_RETURN_DETACH # noqa: F401 — re-exported for back-compat +from .tmux_base import TMUX_TIMEOUT_S # noqa: F401 — re-exported for back-compat +from .tmux_base import TmuxError # noqa: F401 — re-exported for back-compat +from .tmux_base import ( + BaseTmuxBackend, +) - def available(self) -> bool: - return shutil.which("tmux") is not None - def version(self) -> str | None: - if not shutil.which("tmux"): - return None - try: - return self._tmux("-V") - except (MultiplexerError, subprocess.SubprocessError, OSError): - return None +class TmuxMultiplexer(BaseTmuxBackend): + """POSIX tmux backend — inherits the full contract from BaseTmuxBackend.""" diff --git a/src/automator/adapters/tmux_base.py b/src/automator/adapters/tmux_base.py new file mode 100644 index 0000000..1210cf7 --- /dev/null +++ b/src/automator/adapters/tmux_base.py @@ -0,0 +1,298 @@ +"""Shared tmux-family backend base for the terminal-multiplexer seam. + +This module is the **quarantine** for tmux/POSIX-shell knowledge: every tmux +invocation and POSIX-shell trailer lives here (and in its POSIX leaf +:mod:`.tmux_backend`). The point of the split is that a tmux-*family* backend — +an eventual native-Windows "psmux" — can subclass :class:`BaseTmuxBackend` and +override only the single spawn primitive :meth:`BaseTmuxBackend._run` (to tweak +the binary / decoding / timeout) plus the few divergent methods (e.g. the +parked-window trailer), **without editing** :mod:`.tmux_backend`. + +Every method that talks to tmux funnels through :meth:`BaseTmuxBackend._run`, the +one place a subprocess is spawned. See :mod:`.multiplexer` for the contract. +""" + +from __future__ import annotations + +import os +import shlex +import shutil +import subprocess +import time +from pathlib import Path + +from .multiplexer import MultiplexerError, TerminalMultiplexer + +TMUX_TIMEOUT_S = 30 +# Per-window option value (vs a pane id) telling the parked trailer to detach the +# client rather than switch it. Pane ids are %N, so this never collides with one. +PARKED_RETURN_DETACH = "detach" + + +class TmuxError(MultiplexerError): + pass + + +class BaseTmuxBackend(TerminalMultiplexer): + """tmux-family backend: all argv construction and every contract method, with + one overridable subprocess primitive (:meth:`_run`) every call funnels through.""" + + def _run(self, argv: list[str], *, check: bool = True) -> subprocess.CompletedProcess[str]: + """The ONE place tmux is spawned. ``argv`` are the args after ``tmux``. + + With ``check=True`` a non-zero exit raises :class:`TmuxError` (the strict + form behind ``_tmux``); with ``check=False`` the completed process is + returned as-is so callers can apply their own tolerant return-code handling. + A timeout / missing binary always propagates (``TimeoutExpired`` / ``OSError``) + so callers' existing ``try/except`` still fires. + + Subclasses (e.g. a native-Windows psmux) override this to tweak the binary, + decoding (``encoding="utf-8"``), or timeout — and nothing else. + """ + proc = subprocess.run( + ["tmux", *argv], capture_output=True, text=True, timeout=TMUX_TIMEOUT_S + ) + if check and proc.returncode != 0: + raise TmuxError(f"tmux {' '.join(argv[:2])} failed: {proc.stderr.strip()}") + return proc + + def _tmux(self, *args: str) -> str: + return self._run(list(args), check=True).stdout.strip() + + # ----------------------------------------------------------- sessions + + def has_session(self, name: str) -> bool: + # has-session returns nonzero for an absent session (a normal answer, not an + # error), so this can't use check=True. But a timeout or a missing binary + # is a real backend failure: raise the seam type so callers catch it via + # MultiplexerError instead of a raw subprocess error escaping. + try: + probe = self._run(["has-session", "-t", f"={name}"], check=False) + except (subprocess.TimeoutExpired, OSError) as exc: + raise TmuxError(f"tmux has-session failed: {exc}") from exc + return probe.returncode == 0 + + def new_session( + self, name: str, cwd: Path, cols: int | None = None, lines: int | None = None + ) -> None: + # Window 0 is a plain shell so the session survives task windows closing. + # Geometry is pinned only when both dimensions are given (detached agent + # sessions); the control session omits it and takes tmux's default size. + geometry = ["-x", str(cols), "-y", str(lines)] if cols and lines else [] + self._tmux("new-session", "-d", "-s", name, "-c", str(cwd), *geometry) + + def set_session_option(self, name: str, option: str, value: str) -> None: + # set-option has no '=' exact-match form; callers pass a unique full + # session name so plain-name targeting resolves it unambiguously. + self._tmux("set-option", "-t", name, option, value) + + def kill_session(self, name: str) -> None: + # Tolerant of tmux being absent / the session already gone: a best-effort + # teardown backstop, never a hard failure. + if not shutil.which("tmux"): + return + try: + self._run(["kill-session", "-t", f"={name}"], check=False) + except (subprocess.SubprocessError, OSError): + pass + + def list_sessions(self) -> list[str]: + # [] when tmux is missing, no server is running, or the query fails — the + # absence of sessions and the absence of tmux are indistinguishable here + # and callers treat both as "nothing live". + if not shutil.which("tmux"): + return [] + try: + proc = self._run(["list-sessions", "-F", "#{session_name}"], check=False) + except (subprocess.SubprocessError, OSError): + return [] + if proc.returncode != 0: # no server / no sessions + return [] + return [line for line in proc.stdout.splitlines() if line] + + def session_options(self, option: str) -> dict[str, str]: + # Map session name -> value of ``option`` ("" when unset). Same missing + # tmux / no-server tolerance as list_sessions(). + if not shutil.which("tmux"): + return {} + try: + proc = self._run( + ["list-sessions", "-F", f"#{{session_name}}\t#{{{option}}}"], check=False + ) + except (subprocess.SubprocessError, OSError): + return {} + if proc.returncode != 0: # no server / no sessions + return {} + options: dict[str, str] = {} + for line in proc.stdout.splitlines(): + name, _, value = line.partition("\t") + if name: + options[name] = value + return options + + # ------------------------------------------------------------ windows + + def new_window( + self, session: str, name: str, cwd: Path, env: dict[str, str], command: str + ) -> str: + env_args: list[str] = [] + for key, value in env.items(): + env_args += ["-e", f"{key}={value}"] + return self._tmux( + "new-window", + "-t", + f"={session}:", + "-n", + name, + "-c", + str(cwd), + "-P", + "-F", + "#{window_id}", + *env_args, + command, + ) + + def new_parked_window( + self, session: str, name: str, cwd: Path, argv: list[str], return_opt: str + ) -> str: + # The window runs under explicit `sh -c` (the user's login shell may be + # fish); the trailing `read` keeps the exit status inspectable instead of + # tmux closing the window the moment the process exits. After the read the + # return trailer switches an attached client back to its origin pane: + # - return_opt == a pane id (%N): switch that client back there + # (`switch-client -l` is a best-effort fallback when it is gone); + # - return_opt == PARKED_RETURN_DETACH: detach the client so a blocking + # `tmux attach` returns and a suspended TUI resumes; + # - unset/empty: nobody attached interactively -> park as-is. + return_trailer = ( + f"ret=$(tmux show-options -wqv {return_opt} 2>/dev/null); " + f'if [ "$ret" = "{PARKED_RETURN_DETACH}" ]; then tmux detach-client 2>/dev/null; ' + 'elif [ -n "$ret" ]; then ' + 'tmux switch-client -t "$ret" 2>/dev/null || tmux switch-client -l 2>/dev/null; ' + "fi" + ) + inner = shlex.join(argv) + shell = ( + f'{inner}; ec=$?; echo "[bmad-auto exited $ec — press enter]"; ' + f"read -r; {return_trailer}" + ) + return self._tmux( + "new-window", + "-d", + "-P", + "-F", + "#{window_id}", + "-t", + f"={session}:", + "-n", + name, + "-c", + str(cwd), + "sh", + "-c", + shell, + ) + + def list_window_ids(self, session: str) -> list[str]: + # display-message -t exits 0 with empty output, so list the + # session's window ids and check membership instead. + probe = self._run(["list-windows", "-t", f"={session}", "-F", "#{window_id}"], check=False) + if probe.returncode != 0: + return [] + return probe.stdout.split() + + def pipe_pane(self, window_id: str, log_file: Path) -> None: + # A CLI that crashes on launch (bad args, instant auth failure) can take + # its window down before pipe-pane attaches, which races as "can't find + # window". That is not a setup failure, so tolerate it instead of raising. + try: + self._tmux("pipe-pane", "-t", window_id, "-o", f"cat >> {shlex.quote(str(log_file))}") + except TmuxError: + pass + + def send_text(self, window_id: str, text: str) -> None: + self._tmux("send-keys", "-t", window_id, "-l", text) + time.sleep(0.3) # let the TUI ingest the paste before submitting + self._tmux("send-keys", "-t", window_id, "Enter") + + def kill_window(self, target: str) -> None: + self._run(["kill-window", "-t", target], check=False) + + def list_windows(self, session: str, fields: list[str]) -> list[tuple[str, ...]]: + fmt = "\t".join(f"#{{{field}}}" for field in fields) + probe = self._run(["list-windows", "-t", f"={session}", "-F", fmt], check=False) + if probe.returncode != 0: + return [] + rows: list[tuple[str, ...]] = [] + for line in probe.stdout.splitlines(): + parts = line.split("\t") + parts += [""] * (len(fields) - len(parts)) # tolerate unset trailing fields + rows.append(tuple(parts[: len(fields)])) + return rows + + def window_alive(self, session: str, window_id: str) -> bool: + return window_id in self.list_window_ids(session) + + def select_window(self, target: str) -> None: + self._run(["select-window", "-t", target], check=False) + + def set_window_option(self, target: str, option: str, value: str) -> None: + self._run(["set-option", "-w", "-t", target, option, value], check=False) + + def unset_window_option(self, target: str, option: str) -> None: + self._run(["set-option", "-wu", "-t", target, option], check=False) + + def show_window_option(self, target: str, option: str) -> str: + proc = self._run(["show-options", "-wqv", "-t", target, option], check=False) + return proc.stdout.strip() if proc.returncode == 0 else "" + + # ----------------------------------------------------- client / attach + + def attach_target_argv(self, target: str) -> list[str]: + # Inside tmux, nesting an attach is refused, so switch this client + # instead (a `switch-client -l` brings it back). + if os.environ.get("TMUX"): + return ["tmux", "switch-client", "-t", target] + return ["tmux", "attach", "-t", target] + + def current_pane_id(self) -> str | None: + return self._display_message("#{pane_id}") + + def current_window_id(self) -> str | None: + return self._display_message("#{window_id}") + + def current_session(self) -> str | None: + return self._display_message("#{session_name}") + + def _display_message(self, fmt: str) -> str | None: + """Resolve a tmux format string against this process's client, or None + when not inside tmux / tmux is unavailable.""" + try: + proc = self._run(["display-message", "-p", fmt], check=False) + except (subprocess.SubprocessError, OSError): + return None + return proc.stdout.strip() if proc.returncode == 0 else None + + def detach_client(self) -> None: + self._run(["detach-client"], check=False) + + def switch_client(self, target: str, last_fallback: bool = False) -> bool: + proc = self._run(["switch-client", "-t", target], check=False) + if proc.returncode == 0: + return True + if last_fallback: + fb = self._run(["switch-client", "-l"], check=False) + return fb.returncode == 0 + return False + + def available(self) -> bool: + return shutil.which("tmux") is not None + + def version(self) -> str | None: + if not shutil.which("tmux"): + return None + try: + return self._tmux("-V") + except (MultiplexerError, subprocess.SubprocessError, OSError): + return None diff --git a/tests/test_generic_tmux.py b/tests/test_generic_tmux.py index 09b1c5b..b05ac8b 100644 --- a/tests/test_generic_tmux.py +++ b/tests/test_generic_tmux.py @@ -14,7 +14,7 @@ import pytest -from automator.adapters import generic, tmux_backend +from automator.adapters import generic, tmux_base from automator.adapters.base import SessionHandle, SessionResult, SessionSpec from automator.adapters.generic import GenericDevAdapter, GenericTmuxAdapter from automator.adapters.profile import get_profile @@ -75,7 +75,7 @@ def fake_run(argv, **kwargs): rc = 1 if argv[1] == "has-session" else 0 # session missing -> create it return subprocess.CompletedProcess(argv, rc, stdout="", stderr="") - monkeypatch.setattr(tmux_backend.subprocess, "run", fake_run) + monkeypatch.setattr(tmux_base.subprocess, "run", fake_run) adapter._ensure_session(project) assert [c for c in calls if c[1] == "set-option"] == [ diff --git a/tests/test_multiplexer.py b/tests/test_multiplexer.py index b51889b..d82b947 100644 --- a/tests/test_multiplexer.py +++ b/tests/test_multiplexer.py @@ -11,7 +11,7 @@ import pytest -from automator.adapters import tmux_backend +from automator.adapters import tmux_base from automator.adapters.base import SessionSpec from automator.adapters.generic import GenericAdapter from automator.adapters.multiplexer import TerminalMultiplexer @@ -128,7 +128,7 @@ def no_tmux(monkeypatch): def boom(*a, **k): raise AssertionError("GenericAdapter shelled out to tmux directly") - monkeypatch.setattr(tmux_backend.subprocess, "run", boom) + monkeypatch.setattr(tmux_base.subprocess, "run", boom) def _spec(tmp_path): diff --git a/tests/test_portability_guard.py b/tests/test_portability_guard.py index 2a4f254..dd24f56 100644 --- a/tests/test_portability_guard.py +++ b/tests/test_portability_guard.py @@ -25,10 +25,11 @@ # ----------------------------------------------------------------- allowlists -# The single file allowed to shell out to ``tmux`` — the whole-file quarantine -# for tmux/POSIX-shell knowledge. No per-line ack needed: the file *is* the -# sanctioned spot (its module docstring says so). -TMUX_BACKEND = "adapters/tmux_backend.py" +# The files allowed to shell out to ``tmux`` — the whole-file quarantine for +# tmux/POSIX-shell knowledge, split across the shared base (where the spawn +# primitive + argv live) and its POSIX leaf. No per-line ack needed: these files +# *are* the sanctioned spot (their module docstrings say so). +TMUX_BACKENDS = {"adapters/tmux_base.py", "adapters/tmux_backend.py"} # Platform-guarded Unity plugin files that may name a bare POSIX path, each on a # line carrying a `# portability:` ack (and guarded by a sys.platform branch). @@ -197,10 +198,10 @@ def _of(kind: str): def test_no_tmux_invocation_outside_backend(): """Only the tmux backend may build a ``["tmux", ...]`` argv — every other call site goes through the multiplexer seam.""" - offenders = [(rel, ln, txt) for _, rel, ln, txt in _of("tmux") if rel != TMUX_BACKEND] + offenders = [(rel, ln, txt) for _, rel, ln, txt in _of("tmux") if rel not in TMUX_BACKENDS] assert not offenders, ( - "tmux invoked outside adapters/tmux_backend.py — route it through " - "get_multiplexer() instead:\n" + "tmux invoked outside the tmux backend (adapters/tmux_base.py, " + "adapters/tmux_backend.py) — route it through get_multiplexer() instead:\n" + "\n".join(f" {rel}:{ln}: {txt.strip()}" for rel, ln, txt in offenders) ) diff --git a/tests/test_runs.py b/tests/test_runs.py index 4bb1d62..bed9604 100644 --- a/tests/test_runs.py +++ b/tests/test_runs.py @@ -8,7 +8,7 @@ import pytest from automator import runs -from automator.adapters import tmux_backend +from automator.adapters import tmux_base from automator.journal import load_state, save_state from automator.model import RunState @@ -217,14 +217,14 @@ def fake_kill(pid, sig): def test_tmux_sessions_no_tmux(monkeypatch): # tmux_sessions now delegates to the multiplexer backend; patch its seam. - monkeypatch.setattr(tmux_backend.shutil, "which", lambda _name: None) + monkeypatch.setattr(tmux_base.shutil, "which", lambda _name: None) assert runs.tmux_sessions() == [] def test_tmux_sessions_no_server(monkeypatch): - monkeypatch.setattr(tmux_backend.shutil, "which", lambda _name: "/usr/bin/tmux") + monkeypatch.setattr(tmux_base.shutil, "which", lambda _name: "/usr/bin/tmux") monkeypatch.setattr( - tmux_backend.subprocess, + tmux_base.subprocess, "run", lambda *a, **k: subprocess.CompletedProcess(a, 1, stdout="", stderr="no server"), ) diff --git a/tests/test_tui_data.py b/tests/test_tui_data.py index a4fa2c0..e95f16e 100644 --- a/tests/test_tui_data.py +++ b/tests/test_tui_data.py @@ -11,7 +11,7 @@ from conftest import install_bmad_config, write_sprint from automator import deferredwork -from automator.adapters import tmux_backend +from automator.adapters import tmux_base from automator.journal import Journal, save_state from automator.model import RunState from automator.runs import RUNS_DIR, write_pid @@ -144,13 +144,13 @@ def test_finished_beats_stopped(tmp_path): def test_discover_runs_legacy_no_pid_is_unknown(tmp_path, monkeypatch): make_run(tmp_path, "20260611-100000-aaaa") # legacy liveness now flows through the multiplexer backend; patch its seam. - monkeypatch.setattr(tmux_backend.shutil, "which", lambda _: None) + monkeypatch.setattr(tmux_base.shutil, "which", lambda _: None) assert data.discover_runs(tmp_path)[0].status == data.UNKNOWN def test_legacy_run_with_live_tmux_session_is_running(tmp_path, monkeypatch): run_dir = make_run(tmp_path, "20260611-100000-aaaa") - monkeypatch.setattr(tmux_backend.shutil, "which", lambda _: "/usr/bin/tmux") + monkeypatch.setattr(tmux_base.shutil, "which", lambda _: "/usr/bin/tmux") calls = [] def fake_run(argv, **kwargs): @@ -161,7 +161,7 @@ class Proc: return Proc() - monkeypatch.setattr(tmux_backend.subprocess, "run", fake_run) + monkeypatch.setattr(tmux_base.subprocess, "run", fake_run) assert data.discover_runs(tmp_path)[0].status == data.RUNNING assert calls[0][:3] == ["tmux", "has-session", "-t"] assert calls[0][3] == f"=bmad-auto-{run_dir.name}" @@ -172,12 +172,12 @@ def test_legacy_run_liveness_unknown_when_backend_query_fails(tmp_path, monkeypa not a raw subprocess error: a dead query proves nothing about a legacy run, so it degrades to 'unknown' instead of escaping discover_runs() and crashing the TUI.""" make_run(tmp_path, "20260611-100000-aaaa") - monkeypatch.setattr(tmux_backend.shutil, "which", lambda _: "/usr/bin/tmux") + monkeypatch.setattr(tmux_base.shutil, "which", lambda _: "/usr/bin/tmux") def boom(argv, **kwargs): - raise tmux_backend.subprocess.TimeoutExpired(argv, kwargs.get("timeout")) + raise tmux_base.subprocess.TimeoutExpired(argv, kwargs.get("timeout")) - monkeypatch.setattr(tmux_backend.subprocess, "run", boom) + monkeypatch.setattr(tmux_base.subprocess, "run", boom) assert data.discover_runs(tmp_path)[0].status == data.UNKNOWN diff --git a/tests/test_tui_launch.py b/tests/test_tui_launch.py index 6902be5..c83118c 100644 --- a/tests/test_tui_launch.py +++ b/tests/test_tui_launch.py @@ -3,8 +3,9 @@ sanity check of the captured path. The tmux invocations now live in the multiplexer backend (launch drives the -seam), so the tmux subprocess/which seams are patched on ``tmux_backend``; the -captured read-only path still shells out from ``launch`` itself.""" +seam), so the tmux subprocess/which seams are patched on ``tmux_base`` (the +shared backend base where the spawn primitive lives); the captured read-only +path still shells out from ``launch`` itself.""" from __future__ import annotations @@ -15,7 +16,7 @@ import pytest -from automator.adapters import tmux_backend +from automator.adapters import tmux_base from automator.tui import launch @@ -39,8 +40,8 @@ def by_verb(self, verb: str) -> list[list[str]]: @pytest.fixture def fake_run(monkeypatch) -> FakeRun: fake = FakeRun() - monkeypatch.setattr(tmux_backend.subprocess, "run", fake) - monkeypatch.setattr(tmux_backend.shutil, "which", lambda name: f"/usr/bin/{name}") + monkeypatch.setattr(tmux_base.subprocess, "run", fake) + monkeypatch.setattr(tmux_base.shutil, "which", lambda name: f"/usr/bin/{name}") return fake @@ -155,14 +156,14 @@ def test_resume_detached_argv(fake_run, tmp_path: Path): def test_existing_ctl_session_reused(monkeypatch, tmp_path: Path): fake = FakeRun(has_session_rc=0) - monkeypatch.setattr(tmux_backend.subprocess, "run", fake) - monkeypatch.setattr(tmux_backend.shutil, "which", lambda name: f"/usr/bin/{name}") + monkeypatch.setattr(tmux_base.subprocess, "run", fake) + monkeypatch.setattr(tmux_base.shutil, "which", lambda name: f"/usr/bin/{name}") launch.resume_detached(tmp_path, "RID") assert [c[1] for c in fake.calls] == ["has-session", "new-window", "set-option"] def test_launch_without_tmux_raises(monkeypatch, tmp_path: Path): - monkeypatch.setattr(tmux_backend.shutil, "which", lambda name: None) + monkeypatch.setattr(tmux_base.shutil, "which", lambda name: None) assert not launch.tmux_available() with pytest.raises(launch.LaunchError, match="tmux not found"): launch.start_run_detached(tmp_path, "RID") @@ -173,15 +174,15 @@ def failing_run(argv, **kwargs): rc = 1 if argv[1] in ("has-session", "new-window") else 0 return subprocess.CompletedProcess(argv, rc, stdout="", stderr="boom") - monkeypatch.setattr(tmux_backend.subprocess, "run", failing_run) - monkeypatch.setattr(tmux_backend.shutil, "which", lambda name: f"/usr/bin/{name}") + monkeypatch.setattr(tmux_base.subprocess, "run", failing_run) + monkeypatch.setattr(tmux_base.shutil, "which", lambda name: f"/usr/bin/{name}") with pytest.raises(launch.LaunchError, match="new-window.*failed: boom"): launch.start_run_detached(tmp_path, "RID") def test_session_exists(monkeypatch): fake = FakeRun(has_session_rc=0) - monkeypatch.setattr(tmux_backend.subprocess, "run", fake) + monkeypatch.setattr(tmux_base.subprocess, "run", fake) assert launch.session_exists("bmad-auto-x") assert fake.calls[0] == ["tmux", "has-session", "-t", "=bmad-auto-x"] @@ -191,8 +192,8 @@ def fake(argv, **kwargs): out = "run-AAAA\nsweep-RID\nresume-BBBB\n" if argv[1] == "list-windows" else "" return subprocess.CompletedProcess(argv, 0, stdout=out, stderr="") - monkeypatch.setattr(tmux_backend.subprocess, "run", fake) - monkeypatch.setattr(tmux_backend.shutil, "which", lambda name: f"/usr/bin/{name}") + monkeypatch.setattr(tmux_base.subprocess, "run", fake) + monkeypatch.setattr(tmux_base.shutil, "which", lambda name: f"/usr/bin/{name}") assert launch.ctl_window("RID") == "sweep-RID" assert launch.ctl_window("CCCC") is None @@ -201,10 +202,10 @@ def test_ctl_window_no_session_or_tmux(monkeypatch): def fake(argv, **kwargs): return subprocess.CompletedProcess(argv, 1, stdout="", stderr="no session") - monkeypatch.setattr(tmux_backend.subprocess, "run", fake) - monkeypatch.setattr(tmux_backend.shutil, "which", lambda name: f"/usr/bin/{name}") + monkeypatch.setattr(tmux_base.subprocess, "run", fake) + monkeypatch.setattr(tmux_base.shutil, "which", lambda name: f"/usr/bin/{name}") assert launch.ctl_window("RID") is None - monkeypatch.setattr(tmux_backend.shutil, "which", lambda name: None) + monkeypatch.setattr(tmux_base.shutil, "which", lambda name: None) assert launch.ctl_window("RID") is None # no subprocess call attempted @@ -224,7 +225,7 @@ def test_current_pane_id_reads_pane(monkeypatch): def fake(argv, **kwargs): return subprocess.CompletedProcess(argv, 0, stdout="%9\n", stderr="") - monkeypatch.setattr(tmux_backend.subprocess, "run", fake) + monkeypatch.setattr(tmux_base.subprocess, "run", fake) assert launch.current_pane_id() == "%9" @@ -232,7 +233,7 @@ def test_current_pane_id_none_outside_tmux(monkeypatch): def fake(argv, **kwargs): return subprocess.CompletedProcess(argv, 1, stdout="", stderr="no server") - monkeypatch.setattr(tmux_backend.subprocess, "run", fake) + monkeypatch.setattr(tmux_base.subprocess, "run", fake) assert launch.current_pane_id() is None @@ -272,8 +273,8 @@ def fake(argv, **kwargs): killed.append(list(argv)) return subprocess.CompletedProcess(argv, 0, stdout="", stderr="") - monkeypatch.setattr(tmux_backend.subprocess, "run", fake) - monkeypatch.setattr(tmux_backend.shutil, "which", lambda name: f"/usr/bin/{name}") + monkeypatch.setattr(tmux_base.subprocess, "run", fake) + monkeypatch.setattr(tmux_base.shutil, "which", lambda name: f"/usr/bin/{name}") assert launch.prunable_ctl_windows(tmp_path) == ["sweep-20260101-000000-dead"] assert killed == [] # dry-run view kills nothing @@ -285,8 +286,8 @@ def test_prune_ctl_windows_no_session(monkeypatch, tmp_path: Path): def fake(argv, **kwargs): # has-session reports the ctl session is gone return subprocess.CompletedProcess(argv, 1, stdout="", stderr="") - monkeypatch.setattr(tmux_backend.subprocess, "run", fake) - monkeypatch.setattr(tmux_backend.shutil, "which", lambda name: f"/usr/bin/{name}") + monkeypatch.setattr(tmux_base.subprocess, "run", fake) + monkeypatch.setattr(tmux_base.shutil, "which", lambda name: f"/usr/bin/{name}") assert launch.prune_ctl_windows(tmp_path) == [] @@ -329,8 +330,8 @@ def fake(argv, **kwargs): out, rc = "", 0 return subprocess.CompletedProcess(argv, rc, stdout=out, stderr="") - monkeypatch.setattr(tmux_backend.subprocess, "run", fake) - monkeypatch.setattr(tmux_backend.shutil, "which", lambda name: f"/usr/bin/{name}") + monkeypatch.setattr(tmux_base.subprocess, "run", fake) + monkeypatch.setattr(tmux_base.shutil, "which", lambda name: f"/usr/bin/{name}") return calls @@ -365,8 +366,8 @@ def test_return_attached_client_noop_when_unset(monkeypatch): def test_return_attached_client_noop_without_tmux(monkeypatch): ran: list = [] - monkeypatch.setattr(tmux_backend.shutil, "which", lambda name: None) - monkeypatch.setattr(tmux_backend.subprocess, "run", lambda *a, **k: ran.append(a)) + monkeypatch.setattr(tmux_base.shutil, "which", lambda name: None) + monkeypatch.setattr(tmux_base.subprocess, "run", lambda *a, **k: ran.append(a)) assert launch.return_attached_client() is False assert ran == [] # never shells out when tmux is missing From d8213b8afa892624e26c4be43631b4c6837e5ff2 Mon Sep 17 00:00:00 2001 From: pbean Date: Sun, 28 Jun 2026 13:21:58 -0700 Subject: [PATCH 2/9] refactor(adapters): select the multiplexer backend through a registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the hardcoded TmuxMultiplexer construction in get_multiplexer() with a tiny registry: backends self-register via register_multiplexer(name, matches, factory) and are selected by platform match or forced by name through the BMAD_AUTO_MUX_BACKEND env var. The tmux backend registers itself; tmux stays the default everywhere except win32, with a safe tmux fallback, so POSIX behavior is unchanged. Phase B of the adapter-seam close-out: an out-of-tree backend now registers at import with zero core edits, and a bundled one (the in-flight psmux) costs only its own registration line plus one import — no edit to the selection policy. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/automator/adapters/multiplexer.py | 52 +++++++++++++++-- src/automator/adapters/tmux_backend.py | 6 ++ tests/test_backend_registry.py | 80 ++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 5 deletions(-) create mode 100644 tests/test_backend_registry.py diff --git a/src/automator/adapters/multiplexer.py b/src/automator/adapters/multiplexer.py index 2a0f98d..1636f0c 100644 --- a/src/automator/adapters/multiplexer.py +++ b/src/automator/adapters/multiplexer.py @@ -8,14 +8,20 @@ can slot in without the rest of the codebase shelling out to ``tmux`` directly. ``TerminalMultiplexer`` is the contract a backend author implements. Operation -names mirror today's call sites verbatim so the migration is mechanical. Get the -process-wide backend through :func:`get_multiplexer`. +names mirror today's call sites verbatim so the migration is mechanical. Backends +register themselves through :func:`register_multiplexer` (bundled ones from +:func:`_load_builtin_backends`, out-of-tree ones at import time); the process-wide +backend is selected by registry — by platform, or forced by name through the +``BMAD_AUTO_MUX_BACKEND`` env var — and returned by :func:`get_multiplexer`. """ from __future__ import annotations import functools +import os +import sys from abc import ABC, abstractmethod +from collections.abc import Callable from pathlib import Path @@ -171,10 +177,46 @@ def version(self) -> str | None: return None +# (name, matches(platform) -> bool, factory() -> TerminalMultiplexer) +_BACKENDS: list[tuple[str, Callable[[str], bool], Callable[[], TerminalMultiplexer]]] = [] +_BUILTINS_LOADED = False + + +def register_multiplexer( + name: str, + matches: Callable[[str], bool], + factory: Callable[[], TerminalMultiplexer], +) -> None: + """Register a transport backend. ``matches(sys.platform)`` decides automatic + selection; ``name`` is the key for the ``BMAD_AUTO_MUX_BACKEND`` override. + Bundled backends register from :func:`_load_builtin_backends`; an out-of-tree + backend calls this at import time — no core edit required.""" + _BACKENDS.append((name, matches, factory)) + + +def _load_builtin_backends() -> None: + """Import the bundled backends so they self-register. Idempotent and lazy + (called from :func:`get_multiplexer`, not at module import) to stay cycle-safe.""" + global _BUILTINS_LOADED + if _BUILTINS_LOADED: + return + _BUILTINS_LOADED = True + from . import tmux_backend # noqa: F401 — import triggers registration + + @functools.lru_cache(maxsize=1) def get_multiplexer() -> TerminalMultiplexer: - """Return the process-wide terminal multiplexer. The seam where a backend - would later be selected by policy; today it is always tmux.""" - from .tmux_backend import TmuxMultiplexer # lazy import: avoid a cycle + """Return the process-wide terminal multiplexer, selected by registry. + + ``BMAD_AUTO_MUX_BACKEND`` forces a backend by name (test / override hook); + otherwise the first backend whose ``matches(sys.platform)`` is true wins. tmux + is the default fallback, so POSIX behavior is unchanged. Cached — tests that + flip the env var must call ``get_multiplexer.cache_clear()``.""" + forced = os.environ.get("BMAD_AUTO_MUX_BACKEND") + _load_builtin_backends() + for name, matches, factory in _BACKENDS: + if name == forced or (not forced and matches(sys.platform)): + return factory() + from .tmux_backend import TmuxMultiplexer # default fallback return TmuxMultiplexer() diff --git a/src/automator/adapters/tmux_backend.py b/src/automator/adapters/tmux_backend.py index ed2bf5c..244807b 100644 --- a/src/automator/adapters/tmux_backend.py +++ b/src/automator/adapters/tmux_backend.py @@ -18,6 +18,7 @@ import shutil # noqa: F401 — re-exported for callers/tests reaching the spawn seam import subprocess # noqa: F401 — re-exported for callers/tests reaching the spawn seam +from .multiplexer import register_multiplexer from .tmux_base import PARKED_RETURN_DETACH # noqa: F401 — re-exported for back-compat from .tmux_base import TMUX_TIMEOUT_S # noqa: F401 — re-exported for back-compat from .tmux_base import TmuxError # noqa: F401 — re-exported for back-compat @@ -28,3 +29,8 @@ class TmuxMultiplexer(BaseTmuxBackend): """POSIX tmux backend — inherits the full contract from BaseTmuxBackend.""" + + +# tmux is the default everywhere except native Windows (no tmux binary there); +# get_multiplexer still falls back to tmux when no backend matches. +register_multiplexer("tmux", lambda platform: platform != "win32", TmuxMultiplexer) diff --git a/tests/test_backend_registry.py b/tests/test_backend_registry.py new file mode 100644 index 0000000..0e260b0 --- /dev/null +++ b/tests/test_backend_registry.py @@ -0,0 +1,80 @@ +"""Backend-registry selection proof. + +The multiplexer seam selects its transport backend through a registry +(:func:`~automator.adapters.multiplexer.register_multiplexer`) rather than a +hardcoded constructor, so a new OS/backend is a registration — not a core edit. +These tests pin selection: by platform match, by the ``BMAD_AUTO_MUX_BACKEND`` +override, the safe tmux fallback, and the lru_cache gotcha. Backends register a +sentinel ``object()`` factory so a test need not implement the whole ABC. +""" + +import sys + +import pytest + +from automator.adapters import multiplexer as m +from automator.adapters.tmux_backend import TmuxMultiplexer + + +@pytest.fixture +def fresh_registry(monkeypatch): + """Isolate the global registry + lru_cache: snapshot, clear, restore. The env + override is removed so a test opts in explicitly. Teardown restores the real + tmux registry so unrelated tests see normal selection.""" + monkeypatch.delenv("BMAD_AUTO_MUX_BACKEND", raising=False) + saved_backends = list(m._BACKENDS) + saved_loaded = m._BUILTINS_LOADED + m._BACKENDS.clear() + m._BUILTINS_LOADED = False + m.get_multiplexer.cache_clear() + yield m + m._BACKENDS[:] = saved_backends + m._BUILTINS_LOADED = saved_loaded + m.get_multiplexer.cache_clear() + + +def test_default_is_tmux(fresh_registry): + """No override, POSIX host → tmux, selected via the loop's platform match (the + builtin registers ``matches=p != 'win32'``), not just the bottom fallback.""" + assert isinstance(fresh_registry.get_multiplexer(), TmuxMultiplexer) + + +def test_env_override_selects_named_backend(fresh_registry, monkeypatch): + """``BMAD_AUTO_MUX_BACKEND`` resolves a backend by name without monkeypatching + sys.platform. ``matches`` returns False here, so only the name path can pick it.""" + sentinel = object() + fresh_registry.register_multiplexer("fake", lambda p: False, lambda: sentinel) + monkeypatch.setenv("BMAD_AUTO_MUX_BACKEND", "fake") + fresh_registry.get_multiplexer.cache_clear() + assert fresh_registry.get_multiplexer() is sentinel + + +def test_env_override_tmux_returns_tmux(fresh_registry, monkeypatch): + """Forcing the default by name still works (name match short-circuits).""" + monkeypatch.setenv("BMAD_AUTO_MUX_BACKEND", "tmux") + fresh_registry.get_multiplexer.cache_clear() + assert isinstance(fresh_registry.get_multiplexer(), TmuxMultiplexer) + + +def test_unknown_forced_name_falls_back_to_tmux(fresh_registry, monkeypatch): + """An unregistered forced name matches nothing in the loop and lands on the + safe tmux fallback rather than raising.""" + monkeypatch.setenv("BMAD_AUTO_MUX_BACKEND", "nope") + fresh_registry.get_multiplexer.cache_clear() + assert isinstance(fresh_registry.get_multiplexer(), TmuxMultiplexer) + + +def test_match_based_selection_wins_by_order(fresh_registry): + """A backend registered before the builtins whose ``matches`` accepts the + current platform is selected by auto-match (no override), proving platform + selection and registration-order precedence over tmux.""" + sentinel = object() + fresh_registry.register_multiplexer("fake", lambda p: p == sys.platform, lambda: sentinel) + fresh_registry.get_multiplexer.cache_clear() + assert fresh_registry.get_multiplexer() is sentinel + + +def test_get_multiplexer_is_cached(fresh_registry): + """One process-wide instance: repeated calls return the same object.""" + fresh_registry.get_multiplexer.cache_clear() + assert fresh_registry.get_multiplexer() is fresh_registry.get_multiplexer() From c3787907cc62556513200ed37ec5f963e1aa96ba Mon Sep 17 00:00:00 2001 From: pbean Date: Sun, 28 Jun 2026 13:52:48 -0700 Subject: [PATCH 3/9] refactor(process): quarantine pid lifecycle behind a ProcessHost seam MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The kill/liveness primitives branched on sys.platform inline in platform_util, and stop_run had no way to force-kill a wedged engine — so a native-Windows backend (PR #19) would have to edit core. Introduce a ProcessHost seam (terminate / force_kill / is_alive / identity) with per-OS impls and a name-keyed registry mirroring the multiplexer backend selection, so adding an OS is a new subclass + one registration line. stop_run now escalates: an engine that ignores SIGTERM past the grace window is SIGKILL'd — but only while host.identity(pid) still matches the value recorded at stop-time (a pid-reuse guard); otherwise it raises the new StopRunError rather than risk an unrelated process. This is a deliberate POSIX behavior change, pinned by new tests. The graceful-first / single-writer-of- stopped invariant is unchanged. platform_util keeps terminate_pid/pid_alive as thin back-compat shims over the host; detach_kwargs stays put. The TUI worker widens its except to surface StopRunError/ProcessHostError. The portability guard swaps platform_util for process_host in KILL_PROBE_ALLOW and allows its Linux /proc identity read. Unity teardown de-dup is deferred: unity_teardown.py runs under a bare python3 and can't reliably import the package. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/automator/platform_util.py | 86 +++--------- src/automator/process_host.py | 224 ++++++++++++++++++++++++++++++++ src/automator/runs.py | 38 +++++- src/automator/tui/app.py | 5 +- src/automator/tui/data.py | 4 +- tests/test_platform_util.py | 32 +++-- tests/test_portability_guard.py | 10 +- tests/test_process_host.py | 88 +++++++++++++ tests/test_runs.py | 97 ++++++++++++-- 9 files changed, 474 insertions(+), 110 deletions(-) create mode 100644 src/automator/process_host.py create mode 100644 tests/test_process_host.py diff --git a/src/automator/platform_util.py b/src/automator/platform_util.py index a3de7a5..6fab775 100644 --- a/src/automator/platform_util.py +++ b/src/automator/platform_util.py @@ -1,89 +1,33 @@ """Cross-platform process primitives. -Quarantines the handful of POSIX-only process operations the orchestrator and -its plugins rely on, so a future native-Windows backend can slot in without -re-auditing every call site. On Linux/macOS — and WSL, which *is* Linux — these -preserve today's exact behavior; the Windows branches degrade gracefully and are -not yet exercised (no Windows backend ships in this pass). +The pid kill/liveness primitives now live behind the :class:`~automator.process_host.ProcessHost` +seam; ``terminate_pid``/``pid_alive`` remain here as thin back-compat shims that +delegate to it. ``detach_kwargs`` stays a real implementation — it is spawn +configuration, not a process-lifecycle primitive, so it does not belong on the +host. On Linux/macOS — and WSL, which *is* Linux — these preserve today's exact +behavior; the Windows branch degrades gracefully and is not yet exercised. """ from __future__ import annotations -import os -import signal import subprocess import sys -# SIGKILL is absent on Windows; fall back to SIGTERM so attribute access never -# raises. Callers wanting a hard kill should reference this rather than -# signal.SIGKILL directly. -SIGKILL = getattr(signal, "SIGKILL", signal.SIGTERM) # portability: SIGKILL absent on Windows +from .process_host import get_process_host def terminate_pid(pid: int) -> None: - """Politely terminate ``pid``. - - POSIX: ``os.kill(pid, SIGTERM)`` — raises the same ``OSError`` family - (``ProcessLookupError``/``PermissionError``) as before, so callers keep their - existing "already gone / not ours" handling. Windows: degrades to ``taskkill`` - (the closest analogue; not exercised yet — kept guarded for the future - native-Windows backend).""" - if pid <= 0: - # 0/negative target a process group (and 0 is the caller's own group), never - # a specific process — refuse so a corrupt pid file can't signal the orchestrator. - return - if sys.platform == "win32": - # portability: no os.kill(SIGTERM) on Windows — taskkill is the analogue. - subprocess.run([_taskkill(), "/PID", str(pid)], check=False, capture_output=True) - return - os.kill(pid, signal.SIGTERM) + """Politely terminate ``pid``. Back-compat shim over + :meth:`ProcessHost.terminate` — prefer ``get_process_host().terminate(pid)`` + in new code.""" + get_process_host().terminate(pid) def pid_alive(pid: int) -> bool: - """Read-only liveness check for ``pid``. - - POSIX: ``os.kill(pid, 0)`` sends no signal — ``ProcessLookupError`` means the - process is gone, ``PermissionError`` means it exists but isn't ours to signal. - Windows: ``os.kill(pid, 0)`` is **destructive** (it maps to ``TerminateProcess``), - so probe with ``psutil.pid_exists`` instead. This is the single sanctioned - ``os.kill(pid, 0)`` call site in the core — route existence checks here rather - than calling ``os.kill`` directly.""" - if pid <= 0: - # 0/negative target a process group, not a specific process — a corrupt pid - # file must read as "not alive", never as the caller's own group being up. - return False - if sys.platform == "win32": - return _psutil().pid_exists(pid) - try: - os.kill(pid, 0) # portability: read-only existence probe (POSIX); win32 uses psutil above - except ProcessLookupError: - return False - except PermissionError: - return True # exists, just not ours to signal - return True - - -def _psutil(): - """Lazily import psutil (the optional ``non-linux`` extra), used only for the - non-destructive Windows liveness probe. The dep-free core never imports it on - Linux/macOS; raise a clear, actionable error if it's missing where it's needed.""" - try: - import psutil # noqa: PLC0415 (intentional lazy import — keeps the core dep-free) - except ImportError as exc: # pragma: no cover - exercised only on Windows - raise RuntimeError( - f"platform_util: pid liveness on {sys.platform!r} needs psutil; " - "install the optional extra (pip install 'bmad-auto[non-linux]') or run " - "under Linux/WSL" - ) from exc - return psutil - - -def _taskkill() -> str: - """Absolute path to the Windows ``taskkill`` binary. Resolving it from - ``%SystemRoot%\\System32`` rather than invoking ``taskkill`` by name keeps the - Windows process-search order from picking up a same-named executable planted on - PATH or in the working directory.""" - return os.path.join(os.environ.get("SystemRoot", r"C:\Windows"), "System32", "taskkill.exe") + """Read-only liveness check for ``pid``. Back-compat shim over + :meth:`ProcessHost.is_alive` — prefer ``get_process_host().is_alive(pid)`` in + new code.""" + return get_process_host().is_alive(pid) def detach_kwargs() -> dict[str, object]: diff --git a/src/automator/process_host.py b/src/automator/process_host.py new file mode 100644 index 0000000..7ce40c2 --- /dev/null +++ b/src/automator/process_host.py @@ -0,0 +1,224 @@ +"""Cross-platform process-lifecycle primitives behind a single seam. + +The orchestrator needs four operations on a pid it launched: politely stop it, +force-kill it when it ignores that, check whether it is still alive, and — to +guard against pid reuse before a force-kill — read a stable per-process identity. +On POSIX these are ``os.kill`` calls; on Windows they are ``taskkill`` / psutil. +Quarantining them behind :class:`ProcessHost` lets a native-Windows backend slot +in as a new subclass + one registration line, with no edits to the POSIX bodies +or to the callers (`runs.stop_run`, the TUI liveness column). + +On Linux/macOS — and WSL, which *is* Linux — these preserve today's exact +behavior; the Windows branch degrades gracefully and is not yet exercised (no +Windows backend ships in this pass). Selection mirrors the multiplexer registry +in :mod:`automator.adapters.multiplexer`. +""" + +from __future__ import annotations + +import functools +import os +import signal +import subprocess +import sys +from abc import ABC, abstractmethod +from pathlib import Path +from typing import Callable + +# SIGKILL is absent on Windows; fall back to SIGTERM so attribute access never +# raises. The POSIX host references this rather than ``signal.SIGKILL`` directly. +SIGKILL = getattr(signal, "SIGKILL", signal.SIGTERM) # portability: SIGKILL absent on Windows + + +class ProcessHostError(Exception): + """A process-lifecycle operation could not be carried out on this platform.""" + + +class ProcessHost(ABC): + """The four pid operations the orchestrator needs, abstracted over the OS.""" + + @abstractmethod + def terminate(self, pid: int) -> None: + """Politely ask ``pid`` to stop (POSIX SIGTERM / Windows ``taskkill``). + Raises the ``OSError`` family (``ProcessLookupError``/``PermissionError``) + so callers keep their "already gone / not ours" handling.""" + + @abstractmethod + def force_kill(self, pid: int) -> None: + """Forcibly kill ``pid`` (POSIX SIGKILL / Windows ``taskkill /F /T``). The + escalation when ``terminate`` is ignored; only call once identity is + confirmed, never on a possibly-reused pid.""" + + @abstractmethod + def is_alive(self, pid: int) -> bool: + """Read-only liveness check for ``pid`` (no signal sent).""" + + @abstractmethod + def identity(self, pid: int) -> float | None: + """A value that stays constant for the life of ``pid`` but changes if the + pid is reused — a PID-reuse guard for the force-kill escalation. ``None`` + where the platform can't provide one (callers must refuse to force-kill + rather than risk an unrelated process).""" + + +class PosixProcessHost(ProcessHost): + """Linux/macOS/WSL: ``os.kill`` for signalling and the read-only existence + probe; ``/proc`` start-time (Linux) or psutil create-time for identity.""" + + def terminate(self, pid: int) -> None: + if pid <= 0: + # 0/negative target a process group (0 is the caller's own group), never + # a specific process — refuse so a corrupt pid file can't signal us. + return + os.kill(pid, signal.SIGTERM) + + def force_kill(self, pid: int) -> None: + if pid <= 0: + return + os.kill(pid, SIGKILL) + + def is_alive(self, pid: int) -> bool: + if pid <= 0: + # 0/negative target a process group, not a specific process — a corrupt + # pid file must read as "not alive", never as the caller's own group. + return False + try: + os.kill(pid, 0) # portability: read-only existence probe (POSIX); win32 uses psutil + except ProcessLookupError: + return False + except PermissionError: + return True # exists, just not ours to signal + return True + + def identity(self, pid: int) -> float | None: + if pid <= 0: + return None + if sys.platform.startswith("linux"): + return _proc_starttime(pid) + # macOS (and any non-Linux POSIX): no /proc — fall back to psutil if the + # optional extra is present, else give up (None → callers won't force-kill). + try: + return _psutil().Process(pid).create_time() + except Exception: + return None + + +class WindowsProcessHost(ProcessHost): + """Native Windows: ``taskkill`` for signalling, psutil for the non-destructive + liveness probe and create-time identity. Not exercised in this pass — kept so a + psmux-class backend can register it without editing the POSIX bodies above.""" + + def terminate(self, pid: int) -> None: + if pid <= 0: + return + # portability: no os.kill(SIGTERM) on Windows — taskkill is the analogue. + subprocess.run([_taskkill(), "/PID", str(pid)], check=False, capture_output=True) + + def force_kill(self, pid: int) -> None: + if pid <= 0: + return + # portability: SIGKILL has no Windows analogue — taskkill /F /T force-kills + # the process and its child tree. + subprocess.run( + [_taskkill(), "/F", "/T", "/PID", str(pid)], check=False, capture_output=True + ) + + def is_alive(self, pid: int) -> bool: + if pid <= 0: + return False + return _psutil().pid_exists(pid) + + def identity(self, pid: int) -> float | None: + if pid <= 0: + return None + try: + return _psutil().Process(pid).create_time() + except Exception: + return None + + +def _proc_starttime(pid: int) -> float | None: + """The process's start time (clock ticks since boot) from ``/proc//stat`` + field 22 — stable for the life of the pid, so it doubles as a reuse guard. The + comm field (2) is wrapped in parens and may itself contain spaces/parens, so we + split on the last ``)`` before tokenizing the rest. ``None`` if the process is + gone or unreadable.""" + try: + proc = Path("/proc") # portability: Linux-only, guarded by identity()'s platform check + stat = proc.joinpath(str(pid), "stat").read_text(encoding="utf-8") + except OSError: + return None + try: + after_comm = stat[stat.rindex(")") + 1 :].split() + return float(after_comm[19]) # field 22 = index 19 after the comm token + except (ValueError, IndexError): + return None + + +def _psutil(): + """Lazily import psutil (the optional ``non-linux`` extra), used only for the + non-destructive Windows/macOS liveness + identity probes. The dep-free core + never imports it on Linux; raise a clear, actionable error if it's missing where + it's needed.""" + try: + import psutil # noqa: PLC0415 (intentional lazy import — keeps the core dep-free) + except ImportError as exc: # pragma: no cover - exercised only off Linux + raise ProcessHostError( + f"process_host: pid operations on {sys.platform!r} need psutil; " + "install the optional extra (pip install 'bmad-auto[non-linux]') or run " + "under Linux/WSL" + ) from exc + return psutil + + +def _taskkill() -> str: + """Absolute path to the Windows ``taskkill`` binary. Resolving it from + ``%SystemRoot%\\System32`` rather than invoking ``taskkill`` by name keeps the + Windows process-search order from picking up a same-named executable planted on + PATH or in the working directory.""" + return os.path.join(os.environ.get("SystemRoot", r"C:\Windows"), "System32", "taskkill.exe") + + +# (name, matches(platform) -> bool, factory() -> ProcessHost) +_HOSTS: list[tuple[str, Callable[[str], bool], Callable[[], ProcessHost]]] = [] +_BUILTINS_LOADED = False + + +def register_process_host( + name: str, + matches: Callable[[str], bool], + factory: Callable[[], ProcessHost], +) -> None: + """Register a process host. ``matches(sys.platform)`` decides automatic + selection; ``name`` is the key for the ``BMAD_AUTO_PROCESS_HOST`` override. + Bundled hosts register from :func:`_load_builtin_hosts`; an out-of-tree host + calls this at import time — no core edit required.""" + _HOSTS.append((name, matches, factory)) + + +def _load_builtin_hosts() -> None: + """Register the bundled hosts. Idempotent and lazy (called from + :func:`get_process_host`) to match the multiplexer registry's shape; both + builtins live in this module, so there is nothing to import.""" + global _BUILTINS_LOADED + if _BUILTINS_LOADED: + return + _BUILTINS_LOADED = True + register_process_host("posix", lambda platform: platform != "win32", PosixProcessHost) + register_process_host("windows", lambda platform: platform == "win32", WindowsProcessHost) + + +@functools.lru_cache(maxsize=1) +def get_process_host() -> ProcessHost: + """Return the process-wide process host, selected by registry. + + ``BMAD_AUTO_PROCESS_HOST`` forces a host by name (test / override hook); + otherwise the first host whose ``matches(sys.platform)`` is true wins. POSIX is + the default fallback, so behavior on Linux/macOS is unchanged. Cached — tests + that flip the env var must call ``get_process_host.cache_clear()``.""" + forced = os.environ.get("BMAD_AUTO_PROCESS_HOST") + _load_builtin_hosts() + for name, matches, factory in _HOSTS: + if name == forced or (not forced and matches(sys.platform)): + return factory() + return PosixProcessHost() # default fallback diff --git a/src/automator/runs.py b/src/automator/runs.py index 41a1aa9..fc66695 100644 --- a/src/automator/runs.py +++ b/src/automator/runs.py @@ -13,12 +13,19 @@ from .adapters.multiplexer import get_multiplexer from .journal import STATE_FILE, Journal, load_state, save_state from .model import PAUSE_ESCALATION, Phase -from .platform_util import pid_alive, terminate_pid +from .process_host import get_process_host RUNS_DIR = Path(".automator") / "runs" ARCHIVE_DIR = Path(".automator") / "archive" PID_FILE = "engine.pid" + +class StopRunError(Exception): + """A live run could not be stopped — the engine ignored SIGTERM and its pid's + identity can no longer be verified, so force-killing would risk an unrelated + (reused) pid. The caller surfaces this rather than silently marking stopped.""" + + # How long stop_run waits for a signalled engine to exit before falling back to # marking the run stopped itself. _STOP_WAIT_S = 10.0 @@ -120,7 +127,7 @@ def engine_alive(run_dir: Path) -> bool: pid = read_pid(run_dir) if pid is None: return False - return pid_alive(pid) + return get_process_host().is_alive(pid) # ----------------------------------------------------------- stop / delete / archive @@ -213,24 +220,45 @@ def stop_run(run_dir: Path) -> bool: Prefers the engine's own SIGTERM handler so the engine stays the single writer of `stopped` (it marks the run, kills its in-flight agent window, and exits). Falls back to an external kill + mark when there is no live engine - pid, it is a legacy run, or it does not exit in time. + pid, it is a legacy run, or it does not exit in time. A wedged engine that + ignores SIGTERM past the grace window is force-killed — but only while we can + still prove the pid is the same process we signalled (a pid-reuse guard); + otherwise we raise StopRunError rather than risk killing an unrelated process. """ state = load_state(run_dir) if state.finished: return False + host = get_process_host() pid = read_pid(run_dir) + identity = None if pid is not None: + identity = host.identity(pid) # recorded before signalling, for the reuse guard try: - terminate_pid(pid) + host.terminate(pid) except (ProcessLookupError, PermissionError, OSError): pid = None # already gone / not ours — go straight to fallback if pid is not None: deadline = time.monotonic() + _STOP_WAIT_S while time.monotonic() < deadline: - if not pid_alive(pid): + if not host.is_alive(pid): break # exited time.sleep(_STOP_POLL_S) + if host.is_alive(pid): + # still wedged past the grace window — escalate to a force-kill, but + # only if this is provably the same process we signalled (never SIGKILL + # a pid the kernel may have recycled to an unrelated process). + if identity is not None and host.identity(pid) == identity: + try: + host.force_kill(pid) + except (ProcessLookupError, PermissionError, OSError): + pass # raced us to exit — that's the outcome we wanted + else: + raise StopRunError( + f"run {run_dir.name}: engine pid {pid} ignored SIGTERM and its " + "identity can no longer be verified; refusing to force-kill a " + "possibly-reused pid" + ) # the engine clears its agent window itself, but kill the session as a # backstop in case it died before tearing it down kill_session(run_dir.name) diff --git a/src/automator/tui/app.py b/src/automator/tui/app.py index 39f5559..a450946 100644 --- a/src/automator/tui/app.py +++ b/src/automator/tui/app.py @@ -23,7 +23,8 @@ from .. import bmadconfig, decisions, runs, verify from ..journal import load_state from ..policy import POLICY_FILE -from ..runs import RUNS_DIR +from ..process_host import ProcessHostError +from ..runs import RUNS_DIR, StopRunError from . import data, launch from .screens.dashboard import DashboardScreen from .screens.modals import ( @@ -442,7 +443,7 @@ def _stop_run_worker(self, run_id: str, run_dir: Path) -> None: try: runs.stop_run(run_dir) launch.kill_ctl_window(run_id) - except OSError as e: + except (OSError, StopRunError, ProcessHostError) as e: self.call_from_thread(self.notify, f"stop failed: {e}", severity="error") return self.call_from_thread(self.notify, f"run {run_id} stopped") diff --git a/src/automator/tui/data.py b/src/automator/tui/data.py index 64c5735..cf33cfd 100644 --- a/src/automator/tui/data.py +++ b/src/automator/tui/data.py @@ -30,7 +30,7 @@ from ..gates import ATTENTION_FILE from ..journal import JOURNAL_FILE, LOGS_DIR, STATE_FILE, load_state from ..model import RunState -from ..platform_util import pid_alive +from ..process_host import get_process_host from ..runs import PID_FILE, list_run_dirs, session_name # Run statuses shown by the dashboard. @@ -69,7 +69,7 @@ def liveness(run_dir: Path) -> str: except (OSError, ValueError): return _session_liveness(run_dir.name) try: - return "alive" if pid_alive(pid) else "dead" + return "alive" if get_process_host().is_alive(pid) else "dead" except Exception: # never falsely dead — an unexpected probe failure stays 'unknown' return "unknown" diff --git a/tests/test_platform_util.py b/tests/test_platform_util.py index 858691b..c6a5e14 100644 --- a/tests/test_platform_util.py +++ b/tests/test_platform_util.py @@ -1,31 +1,35 @@ -"""Tests for the cross-platform process primitives.""" +"""Tests for the back-compat shims over the ProcessHost seam. + +The kill/liveness bodies (and their pid<=0 guards) now live in +``automator.process_host`` — see ``test_process_host.py``. These cover only that +the legacy ``platform_util`` entry points still delegate, plus the real +``detach_kwargs`` that stayed behind.""" from __future__ import annotations import os +import sys + +import pytest from automator import platform_util -def test_pid_alive_true_for_self(): +def test_pid_alive_shim_true_for_self(): assert platform_util.pid_alive(os.getpid()) is True -def test_pid_alive_rejects_non_positive(monkeypatch): - # 0/negative would target a process group via os.kill — the guard must short - # out before os.kill is ever reached. - def _boom(*_a, **_k): # pragma: no cover - must not run - raise AssertionError("os.kill must not be called for pid <= 0") - - monkeypatch.setattr(platform_util.os, "kill", _boom) +def test_pid_alive_shim_false_for_non_positive(): assert platform_util.pid_alive(0) is False assert platform_util.pid_alive(-1) is False -def test_terminate_pid_rejects_non_positive(monkeypatch): - def _boom(*_a, **_k): # pragma: no cover - must not run - raise AssertionError("os.kill must not be called for pid <= 0") - - monkeypatch.setattr(platform_util.os, "kill", _boom) +def test_terminate_pid_shim_noop_for_non_positive(): + # delegates to the host, whose pid<=0 guard short-circuits before any signal platform_util.terminate_pid(0) # no raise, no signal platform_util.terminate_pid(-42) + + +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX detach branch") +def test_detach_kwargs_posix(): + assert platform_util.detach_kwargs() == {"start_new_session": True} diff --git a/tests/test_portability_guard.py b/tests/test_portability_guard.py index dd24f56..a10651e 100644 --- a/tests/test_portability_guard.py +++ b/tests/test_portability_guard.py @@ -31,11 +31,13 @@ # *are* the sanctioned spot (their module docstrings say so). TMUX_BACKENDS = {"adapters/tmux_base.py", "adapters/tmux_backend.py"} -# Platform-guarded Unity plugin files that may name a bare POSIX path, each on a -# line carrying a `# portability:` ack (and guarded by a sys.platform branch). +# Platform-guarded files that may name a bare POSIX path, each on a line carrying +# a `# portability:` ack (and guarded by a sys.platform branch). process_host.py's +# Linux identity reader walks `/proc//stat`. PATH_ALLOW = { "data/plugins/unity/unity_cleanup.py", "data/plugins/unity/unity_teardown.py", + "process_host.py", } # The two detach helpers that legitimately request POSIX `start_new_session`. @@ -47,9 +49,9 @@ # `os.kill(pid, 0)` is a read-only existence probe on POSIX but *destructive* on # Windows (it maps to TerminateProcess). Confine it to the platform-guarded # liveness helpers, each on a line carrying a `# portability:` ack; everything -# else routes through `platform_util.pid_alive`. +# else routes through the ProcessHost seam (`get_process_host().is_alive`). KILL_PROBE_ALLOW = { - "platform_util.py", + "process_host.py", "data/plugins/unity/unity_teardown.py", } diff --git a/tests/test_process_host.py b/tests/test_process_host.py new file mode 100644 index 0000000..0cf7c88 --- /dev/null +++ b/tests/test_process_host.py @@ -0,0 +1,88 @@ +"""Tests for the cross-platform process-lifecycle seam.""" + +from __future__ import annotations + +import os +import sys + +import pytest + +from automator import process_host +from automator.process_host import PosixProcessHost, WindowsProcessHost, get_process_host + + +@pytest.fixture(autouse=True) +def _clear_host_cache(): + # get_process_host is lru_cached and env-driven; isolate every case. + get_process_host.cache_clear() + yield + get_process_host.cache_clear() + + +@pytest.fixture +def host(): + return PosixProcessHost() + + +def test_is_alive_true_for_self(host): + assert host.is_alive(os.getpid()) is True + + +def test_is_alive_rejects_non_positive(host, monkeypatch): + # 0/negative would target a process group via os.kill — the guard must short + # out before os.kill is ever reached. + def _boom(*_a, **_k): # pragma: no cover - must not run + raise AssertionError("os.kill must not be called for pid <= 0") + + monkeypatch.setattr(process_host.os, "kill", _boom) + assert host.is_alive(0) is False + assert host.is_alive(-1) is False + + +def test_terminate_rejects_non_positive(host, monkeypatch): + def _boom(*_a, **_k): # pragma: no cover - must not run + raise AssertionError("os.kill must not be called for pid <= 0") + + monkeypatch.setattr(process_host.os, "kill", _boom) + host.terminate(0) # no raise, no signal + host.terminate(-42) + + +def test_force_kill_rejects_non_positive(host, monkeypatch): + def _boom(*_a, **_k): # pragma: no cover - must not run + raise AssertionError("os.kill must not be called for pid <= 0") + + monkeypatch.setattr(process_host.os, "kill", _boom) + host.force_kill(0) # no raise, no signal + host.force_kill(-42) + + +@pytest.mark.skipif(not sys.platform.startswith("linux"), reason="identity via /proc is Linux-only") +def test_identity_stable_and_present_for_self(host): + first = host.identity(os.getpid()) + second = host.identity(os.getpid()) + assert isinstance(first, float) + assert first == second # stable for the life of the pid → a usable reuse guard + + +def test_identity_none_for_non_positive(host): + assert host.identity(0) is None + assert host.identity(-1) is None + + +def test_default_host_is_posix_on_posix(monkeypatch): + monkeypatch.delenv("BMAD_AUTO_PROCESS_HOST", raising=False) + get_process_host.cache_clear() + assert isinstance(get_process_host(), PosixProcessHost) + + +def test_env_override_selects_by_name(monkeypatch): + # The registry selects by name without monkeypatching sys.platform — the hook + # PR #19's WindowsProcessHost registration relies on. + monkeypatch.setenv("BMAD_AUTO_PROCESS_HOST", "posix") + get_process_host.cache_clear() + assert isinstance(get_process_host(), PosixProcessHost) + + monkeypatch.setenv("BMAD_AUTO_PROCESS_HOST", "windows") + get_process_host.cache_clear() + assert isinstance(get_process_host(), WindowsProcessHost) diff --git a/tests/test_runs.py b/tests/test_runs.py index bed9604..616e323 100644 --- a/tests/test_runs.py +++ b/tests/test_runs.py @@ -41,6 +41,34 @@ def _dead_pid() -> int: return proc.pid +class _FakeHost: + """A ProcessHost stand-in for driving stop_run's escalation deterministically + without spawning real processes. ``alive`` / ``identity`` may be a value or a + zero-arg callable (so they can change between the stop-time read and the + post-grace check).""" + + def __init__(self, *, alive, identity=1.0, on_terminate=None): + self._alive = alive + self._identity = identity + self.on_terminate = on_terminate + self.terminated: list[int] = [] + self.force_killed: list[int] = [] + + def terminate(self, pid): + self.terminated.append(pid) + if self.on_terminate is not None: + self.on_terminate(pid) + + def force_kill(self, pid): + self.force_killed.append(pid) + + def is_alive(self, pid): + return self._alive() if callable(self._alive) else self._alive + + def identity(self, pid): + return self._identity() if callable(self._identity) else self._identity + + def test_list_run_dirs_sorted_and_filtered(tmp_path): _make_run(tmp_path, "20260611-120000-bbbb") _make_run(tmp_path, "20260610-090000-aaaa") @@ -190,28 +218,73 @@ def test_stop_run_respects_engine_written_stopped(tmp_path, monkeypatch): trusts it and does not re-journal a fallback entry.""" monkeypatch.setattr(runs, "kill_session", lambda _rid: None) run_dir = _make_state_run(tmp_path, "r1") - proc = subprocess.Popen(["sleep", "30"]) - (run_dir / "engine.pid").write_text(str(proc.pid)) - - real_kill = os.kill + (run_dir / "engine.pid").write_text("4242") - def fake_kill(pid, sig): + def _mark_stopped(_pid): # emulate the engine handler marking stopped, then dying on SIGTERM - if pid == proc.pid and sig != 0: - st = load_state(run_dir) - st.stopped = True - save_state(run_dir, st) - return real_kill(pid, sig) + st = load_state(run_dir) + st.stopped = True + save_state(run_dir, st) - monkeypatch.setattr(runs.os, "kill", fake_kill) + host = _FakeHost(alive=False, on_terminate=_mark_stopped) + monkeypatch.setattr(runs, "get_process_host", lambda: host) assert runs.stop_run(run_dir) is True - proc.wait(timeout=5) assert load_state(run_dir).stopped is True + assert host.force_killed == [] # exited gracefully — no escalation # trusted the engine: no fallback journal entry written journal = run_dir / "journal.jsonl" assert not journal.exists() or "fallback" not in journal.read_text() +def test_stop_run_force_kills_wedged_engine(tmp_path, monkeypatch): + """An engine that ignores SIGTERM past the grace window is force-killed, then + marked stopped — as long as its pid identity still matches what we recorded.""" + monkeypatch.setattr(runs, "kill_session", lambda _rid: None) + monkeypatch.setattr(runs, "_STOP_WAIT_S", 0.05) + monkeypatch.setattr(runs, "_STOP_POLL_S", 0.01) + run_dir = _make_state_run(tmp_path, "r1") + (run_dir / "engine.pid").write_text("4242") + + host = _FakeHost(alive=True, identity=123.0) # never exits, identity stable + monkeypatch.setattr(runs, "get_process_host", lambda: host) + assert runs.stop_run(run_dir) is True + assert host.force_killed == [4242] + assert load_state(run_dir).stopped is True + + +def test_stop_run_refuses_force_kill_on_identity_mismatch(tmp_path, monkeypatch): + """If the pid is still 'alive' but its identity changed during the grace window + (possible pid reuse), refuse to force-kill and raise StopRunError instead.""" + monkeypatch.setattr(runs, "kill_session", lambda _rid: None) + monkeypatch.setattr(runs, "_STOP_WAIT_S", 0.05) + monkeypatch.setattr(runs, "_STOP_POLL_S", 0.01) + run_dir = _make_state_run(tmp_path, "r1") + (run_dir / "engine.pid").write_text("4242") + + identities = iter([123.0, 999.0]) # recorded at stop-time, then changed + host = _FakeHost(alive=True, identity=lambda: next(identities)) + monkeypatch.setattr(runs, "get_process_host", lambda: host) + with pytest.raises(runs.StopRunError): + runs.stop_run(run_dir) + assert host.force_killed == [] + + +def test_stop_run_refuses_force_kill_without_identity(tmp_path, monkeypatch): + """On a platform that can't provide an identity (None), a wedged engine can't + be safely force-killed — raise StopRunError rather than risk a reused pid.""" + monkeypatch.setattr(runs, "kill_session", lambda _rid: None) + monkeypatch.setattr(runs, "_STOP_WAIT_S", 0.05) + monkeypatch.setattr(runs, "_STOP_POLL_S", 0.01) + run_dir = _make_state_run(tmp_path, "r1") + (run_dir / "engine.pid").write_text("4242") + + host = _FakeHost(alive=True, identity=None) + monkeypatch.setattr(runs, "get_process_host", lambda: host) + with pytest.raises(runs.StopRunError): + runs.stop_run(run_dir) + assert host.force_killed == [] + + # ---------------------------------------------------------------- prune sessions From 2db2d31fbc710afd525f4aca0af01525636e39fd Mon Sep 17 00:00:00 2001 From: pbean Date: Sun, 28 Jun 2026 14:25:51 -0700 Subject: [PATCH 4/9] refactor(process): seam the hook interpreter and validate preflight MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hook-registration sites in install/probe hardcoded `python3`, and `cmd_validate` hardcoded a `tmux` PATH check — two more axes a native backend (psmux PR #19) would have to branch on `sys.platform` to cross. Route both through the platform-selected seams so a new OS registers rather than edits core: - Add `ProcessHost.hook_interpreter()` (POSIX `python3`; Windows `uv run --no-project python`); install/probe interpolate it instead of a `python3` literal. POSIX output is byte-for-byte unchanged. - Extract `_platform_preflight()` in cli.py: ask the multiplexer backend (`available()`/`version()`) and name the process host, replacing the hardcoded `tmux` probe in `cmd_validate`. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/automator/cli.py | 42 +++++++++++++++++++++++++++++---- src/automator/install.py | 9 ++++--- src/automator/probe.py | 4 +++- src/automator/process_host.py | 16 +++++++++++++ tests/test_cli.py | 44 +++++++++++++++++++++++++++++++++++ tests/test_install.py | 17 ++++++++++++++ tests/test_process_host.py | 20 ++++++++++++++++ 7 files changed, 143 insertions(+), 9 deletions(-) diff --git a/src/automator/cli.py b/src/automator/cli.py index 8dc563a..12f035e 100644 --- a/src/automator/cli.py +++ b/src/automator/cli.py @@ -108,6 +108,36 @@ def _make_adapters(project: Path, run_dir: Path, policy) -> dict[str, CodingCLIA # ----------------------------------------------------------------- commands +def _platform_preflight() -> tuple[list[str], list[str]]: + """Probe the platform-selected seams — the terminal multiplexer and the process + host — for `cmd_validate`, returning ``(notes, problems)``. + + A backend reports its own readiness through ``available()`` / ``version()``, so + a new OS or transport surfaces here by *registering* rather than by adding a + ``sys.platform`` branch to validate. The process host is named so a + misselection (e.g. the Windows host picked on Linux) is visible at a glance. + """ + from .adapters.multiplexer import get_multiplexer + from .process_host import get_process_host + + notes: list[str] = [] + problems: list[str] = [] + + backend = get_multiplexer() + label = type(backend).__name__ + if backend.available(): + version = backend.version() + notes.append(f"multiplexer {label} available" + (f" ({version})" if version else "")) + else: + problems.append( + f"multiplexer {label} unavailable — its transport binary is not on PATH; " + f"see `bmad-auto diagnose`" + ) + + notes.append(f"process host: {type(get_process_host()).__name__}") + return notes, problems + + def cmd_validate(args: argparse.Namespace) -> int: project = _project(args) problems: list[str] = [] @@ -160,8 +190,11 @@ def cmd_validate(args: argparse.Namespace) -> int: except verify.GitError as e: problems.append(f"git check failed: {e}") - tools = ("tmux", *dict.fromkeys(p.binary for p in profiles)) - for tool in tools: + pf_notes, pf_problems = _platform_preflight() + notes.extend(pf_notes) + problems.extend(pf_problems) + + for tool in dict.fromkeys(p.binary for p in profiles): if shutil.which(tool): notes.append(f"{tool} found") else: @@ -579,7 +612,7 @@ def cmd_resolve(args: argparse.Namespace) -> int: return 1 if not produced: print( - f"no resolution recorded for {story_key} " f"(agent did not write resolution.json)", + f"no resolution recorded for {story_key} (agent did not write resolution.json)", file=sys.stderr, ) @@ -939,8 +972,7 @@ def cmd_tui(args: argparse.Namespace) -> int: except ModuleNotFoundError as e: if (e.name or "").partition(".")[0] in ("textual", "tomlkit"): print( - "error: the TUI requires optional dependencies — " - "uv tool install 'bmad-auto[tui]'", + "error: the TUI requires optional dependencies — uv tool install 'bmad-auto[tui]'", file=sys.stderr, ) return 1 diff --git a/src/automator/install.py b/src/automator/install.py index 43aac50..8f25af3 100644 --- a/src/automator/install.py +++ b/src/automator/install.py @@ -26,6 +26,7 @@ from .adapters.profile import ALIASES, CLIProfile, ProfileError, load_profiles from .policy import POLICY_TEMPLATE +from .process_host import get_process_host HOOK_SCRIPT_REL = ".automator/bmad_auto_hook.py" # Dedup marker: matches any bmad-auto-managed hook command — both the signal @@ -92,11 +93,12 @@ def missing_base_skills(project: Path, trees: Sequence[str]) -> list[str]: def _hook_command(project: Path, profile: CLIProfile, canonical_event: str) -> str: + interp = get_process_host().hook_interpreter() if profile.hooks.dialect == "claude-settings-json": - return f'python3 "$CLAUDE_PROJECT_DIR"/{HOOK_SCRIPT_REL} {canonical_event}' + return f'{interp} "$CLAUDE_PROJECT_DIR"/{HOOK_SCRIPT_REL} {canonical_event}' # Codex/Gemini expose no $CLAUDE_PROJECT_DIR equivalent to hook commands; # bake the absolute path at init time. - return f"python3 {shlex.quote(str(project / HOOK_SCRIPT_REL))} {canonical_event}" + return f"{interp} {shlex.quote(str(project / HOOK_SCRIPT_REL))} {canonical_event}" def _hook_entry(dialect: str, command: str) -> dict: @@ -344,8 +346,9 @@ def provision_worktree( config = json.loads(config_path.read_text(encoding="utf-8")) except json.JSONDecodeError: config = {} + interp = get_process_host().hook_interpreter() registrations = { - native: f"python3 {shlex.quote(str(relay))} {canonical}" + native: f"{interp} {shlex.quote(str(relay))} {canonical}" for native, canonical in profile.hooks.events.items() } config, changed = merge_hooks(config, registrations, profile.hooks.dialect) diff --git a/src/automator/probe.py b/src/automator/probe.py index 50de52e..3acc609 100644 --- a/src/automator/probe.py +++ b/src/automator/probe.py @@ -39,6 +39,7 @@ from .adapters.multiplexer import MultiplexerError, get_multiplexer from .adapters.profile import CLIProfile from .install import merge_hooks +from .process_host import get_process_host from .signals import SignalWatcher from .tokens import _jsonl_entries, read_usage @@ -504,8 +505,9 @@ def probe( hook_src = resources.files("automator.data").joinpath(PROBE_HOOK_NAME) hook_path = tmpdir / PROBE_HOOK_NAME hook_path.write_text(hook_src.read_text(encoding="utf-8"), encoding="utf-8") + interp = get_process_host().hook_interpreter() registrations = { - native: f"python3 {shlex.quote(str(hook_path))} {canonical}" + native: f"{interp} {shlex.quote(str(hook_path))} {canonical}" for native, canonical in profile.hooks.events.items() } config, _ = merge_hooks({}, registrations, profile.hooks.dialect) diff --git a/src/automator/process_host.py b/src/automator/process_host.py index 7ce40c2..24ff266 100644 --- a/src/automator/process_host.py +++ b/src/automator/process_host.py @@ -60,6 +60,14 @@ def identity(self, pid: int) -> float | None: where the platform can't provide one (callers must refuse to force-kill rather than risk an unrelated process).""" + @abstractmethod + def hook_interpreter(self) -> str: + """The command prefix that runs a bmad-auto python hook script on this + host, interpolated into the hook registrations `install`/`probe` write + (the script path + canonical event are appended by the caller). POSIX runs + the ``python3`` on PATH; a Windows host overrides it (no ``python3`` there) + so hook registration never branches on ``sys.platform`` at the call site.""" + class PosixProcessHost(ProcessHost): """Linux/macOS/WSL: ``os.kill`` for signalling and the read-only existence @@ -102,6 +110,9 @@ def identity(self, pid: int) -> float | None: except Exception: return None + def hook_interpreter(self) -> str: + return "python3" + class WindowsProcessHost(ProcessHost): """Native Windows: ``taskkill`` for signalling, psutil for the non-destructive @@ -136,6 +147,11 @@ def identity(self, pid: int) -> float | None: except Exception: return None + def hook_interpreter(self) -> str: + # Windows ships no `python3` launcher; `uv run --no-project python` resolves + # an interpreter without activating a project venv (hooks fire detached). + return "uv run --no-project python" + def _proc_starttime(pid: int) -> float | None: """The process's start time (clock ticks since boot) from ``/proc//stat`` diff --git a/tests/test_cli.py b/tests/test_cli.py index 80bc611..cc4a99c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -822,3 +822,47 @@ def test_diagnose_legend_written_locally(project, tmp_path): legend = json.loads(legend_file.read_text()) assert STORY_KEY in legend.values() # legend reverses pseudonyms locally assert STORY_KEY not in out_file.read_text() # but the dump never carries it + + +# ---- validate platform preflight (routes through the multiplexer + host seams) ---- + + +class _FakeBackend: + def __init__(self, ok, version=None): + self._ok, self._version = ok, version + + def available(self): + return self._ok + + def version(self): + return self._version + + +class _FakeHost: + pass + + +def _patch_preflight(monkeypatch, backend): + from automator import process_host as ph_mod + from automator.adapters import multiplexer as mux_mod + + monkeypatch.setattr(mux_mod, "get_multiplexer", lambda: backend) + monkeypatch.setattr(ph_mod, "get_process_host", lambda: _FakeHost()) + + +def test_platform_preflight_reports_available_backend(monkeypatch): + # An available backend reports through available()/version() — no sys.platform + # branch — and the selected process host is named for visibility. + _patch_preflight(monkeypatch, _FakeBackend(ok=True, version="tmux 3.4")) + notes, problems = cli._platform_preflight() + assert not problems + assert any("_FakeBackend" in n and "tmux 3.4" in n for n in notes) + assert any("process host" in n and "_FakeHost" in n for n in notes) + + +def test_platform_preflight_flags_unavailable_backend(monkeypatch): + # A backend whose transport binary is absent surfaces here as a problem, so a + # new OS registers a backend rather than inlining a win32 block in validate. + _patch_preflight(monkeypatch, _FakeBackend(ok=False)) + notes, problems = cli._platform_preflight() + assert any("unavailable" in p for p in problems) diff --git a/tests/test_install.py b/tests/test_install.py index ea676f2..ff69a1a 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -147,6 +147,23 @@ def test_install_into_full(tmp_path): assert final_gitignore.count(".automator/cache/") == 1 +def test_hook_command_uses_selected_process_host(tmp_path, monkeypatch): + # The hook interpreter is platform-selected: forcing the Windows host swaps the + # registered command's prefix without `install` branching on sys.platform. + from automator.process_host import get_process_host + + monkeypatch.setenv("BMAD_AUTO_PROCESS_HOST", "windows") + get_process_host.cache_clear() + try: + assert install_into(tmp_path) == 0 + settings = json.loads((tmp_path / ".claude" / "settings.json").read_text()) + cmd = settings["hooks"]["Stop"][0]["hooks"][0]["command"] + assert cmd.startswith("uv run --no-project python ") + finally: + monkeypatch.delenv("BMAD_AUTO_PROCESS_HOST", raising=False) + get_process_host.cache_clear() + + def test_install_into_multiple_clis(tmp_path): assert install_into(tmp_path, clis=("codex", "gemini")) == 0 diff --git a/tests/test_process_host.py b/tests/test_process_host.py index 0cf7c88..28b391d 100644 --- a/tests/test_process_host.py +++ b/tests/test_process_host.py @@ -86,3 +86,23 @@ def test_env_override_selects_by_name(monkeypatch): monkeypatch.setenv("BMAD_AUTO_PROCESS_HOST", "windows") get_process_host.cache_clear() assert isinstance(get_process_host(), WindowsProcessHost) + + +def test_hook_interpreter_is_python3_on_posix(host): + # Hook registrations (install/probe) interpolate this prefix; POSIX keeps the + # historical `python3` byte-for-byte so existing configs stay valid. + assert PosixProcessHost().hook_interpreter() == "python3" + + +def test_hook_interpreter_windows_resolves_without_project_venv(): + # Windows has no `python3` launcher — `uv run` resolves an interpreter, and + # `--no-project` keeps it from activating a project venv for a detached hook. + assert WindowsProcessHost().hook_interpreter() == "uv run --no-project python" + + +def test_hook_interpreter_routed_through_selected_host(monkeypatch): + # The env override drives the prefix end-to-end, so a Windows host changes the + # registered hook command with no `sys.platform` branch at the call site. + monkeypatch.setenv("BMAD_AUTO_PROCESS_HOST", "windows") + get_process_host.cache_clear() + assert get_process_host().hook_interpreter() == "uv run --no-project python" From 6b4b555fa5557152069108bb506df6c8fd92ec0b Mon Sep 17 00:00:00 2001 From: pbean Date: Sun, 28 Jun 2026 14:25:59 -0700 Subject: [PATCH 5/9] refactor(unity): route teardown pid lifecycle through ProcessHost MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Unity teardown re-implemented `_terminate_pid`/`_hard_kill_pid`/ `_alive`/`_taskkill` — the duplication the ProcessHost seam was meant to retire. Delegate to `get_process_host()` and keep only the Unity-specific `_lingering_pids*` worktree-discovery in the file. To make the import safe from the spawned helper, run plugin scripts under the orchestrator's own interpreter (`sys.executable`) instead of a PATH-resolved `python3` — otherwise a pipx-style install (PATH `python3` lacks the package) would crash the best-effort teardown. Deltas: Windows force-kill now adds `/T` (reaps the leaked MCP server's child tree); `pid <= 0` is guarded. The sweep keeps its exact terminate -> poll -> force-kill-survivors shape. Drops the now-unused unity entry from the portability guard's KILL_PROBE_ALLOW. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../data/plugins/unity/unity_plugin.py | 6 +- .../data/plugins/unity/unity_teardown.py | 60 +++---------------- tests/test_engine_plugin.py | 46 ++++++++++++++ tests/test_portability_guard.py | 4 +- 4 files changed, 60 insertions(+), 56 deletions(-) diff --git a/src/automator/data/plugins/unity/unity_plugin.py b/src/automator/data/plugins/unity/unity_plugin.py index d9b37ac..57f9f60 100644 --- a/src/automator/data/plugins/unity/unity_plugin.py +++ b/src/automator/data/plugins/unity/unity_plugin.py @@ -25,6 +25,7 @@ from __future__ import annotations import subprocess +import sys from pathlib import Path from typing import Any @@ -161,7 +162,10 @@ def _run_script(self, name: str, ctx, *, timeout: int) -> tuple[int, str]: cwd = ctx.worktree or ctx.repo_root or None try: proc = subprocess.run( # nosec B603 - operator-enabled engine plugin script - ["python3", str(script)], + # Our own interpreter, not a PATH-resolved `python3`: the helper now + # imports `automator.process_host`, which must be importable here even + # under a pipx-style install where PATH `python3` lacks the package. + [sys.executable, str(script)], cwd=cwd, env=env, capture_output=True, diff --git a/src/automator/data/plugins/unity/unity_teardown.py b/src/automator/data/plugins/unity/unity_teardown.py index 43fd10e..89a90f2 100644 --- a/src/automator/data/plugins/unity/unity_teardown.py +++ b/src/automator/data/plugins/unity/unity_teardown.py @@ -39,23 +39,12 @@ import os import shutil -import signal import subprocess import sys import time from pathlib import Path -# SIGKILL is absent on Windows; fall back to SIGTERM so the attribute access never -# raises. Linux/macOS/WSL keep today's hard-kill behavior exactly. -_SIGKILL = getattr(signal, "SIGKILL", signal.SIGTERM) # portability: SIGKILL absent on Windows - - -def _taskkill() -> str: - """Absolute path to the Windows ``taskkill`` binary. Resolving it from - ``%SystemRoot%\\System32`` rather than invoking ``taskkill`` by name keeps the - Windows process-search order from picking up a same-named executable planted on - PATH or in the working directory.""" - return os.path.join(os.environ.get("SystemRoot", r"C:\Windows"), "System32", "taskkill.exe") +from automator.process_host import get_process_host def _psutil(): @@ -73,46 +62,11 @@ def _psutil(): return psutil -def _terminate_pid(pid: int) -> None: - """Politely terminate ``pid``. POSIX: ``os.kill(pid, SIGTERM)`` — same OSError - family (ProcessLookupError/PermissionError) as before, so the caller's handling - is unchanged. Windows: ``taskkill`` is the analogue (not exercised yet).""" - if sys.platform == "win32": - # portability: no os.kill(SIGTERM) on Windows — taskkill is the analogue. - subprocess.run([_taskkill(), "/PID", str(pid)], check=False, capture_output=True) - return - os.kill(pid, signal.SIGTERM) - - -def _hard_kill_pid(pid: int) -> None: - """Force-kill ``pid``. POSIX: ``os.kill(pid, SIGKILL)`` (SIGTERM where SIGKILL is - absent). Windows: ``taskkill /F`` (not exercised yet).""" - if sys.platform == "win32": - # portability: SIGKILL has no Windows analogue — taskkill /F force-kills. - subprocess.run([_taskkill(), "/F", "/PID", str(pid)], check=False, capture_output=True) - return - os.kill(pid, _SIGKILL) - - def _worktree() -> Path | None: wt = os.environ.get("BMAD_AUTO_WORKTREE") return Path(wt) if wt else None -def _alive(pid: int) -> bool: - if sys.platform == "win32": - # portability: os.kill(pid, 0) is destructive on Windows (TerminateProcess), - # so use psutil for a read-only liveness check. - return _psutil().pid_exists(pid) - try: - os.kill(pid, 0) # portability: read-only existence probe (POSIX); win32 uses psutil above - except ProcessLookupError: - return False - except PermissionError: - return True # exists, just not ours to signal - return True - - # Process basenames we reap when bound to the worktree path: the Unity Editor # binary and the local MCP HTTP server the plugin spawns as a child. _TARGET_BASENAMES = ("unity", "gamedev-mcp-server") @@ -205,20 +159,21 @@ def _force_kill_lingering(worktree: Path) -> int: f"running ({pids}); hard-killing", file=sys.stderr, ) + host = get_process_host() for pid in pids: try: - _terminate_pid(pid) + host.terminate(pid) except OSError: pass # give them a few seconds to exit politely, then hard-kill survivors for _ in range(20): - if not any(_alive(p) for p in pids): + if not any(host.is_alive(p) for p in pids): break time.sleep(0.5) for pid in pids: - if _alive(pid): + if host.is_alive(pid): try: - _hard_kill_pid(pid) + host.force_kill(pid) except OSError: pass return len(pids) @@ -273,8 +228,7 @@ def main() -> int: ) else: print( - f"unity_teardown: unknown BMAD_AUTO_ENGINE_MCP={mcp!r} " - "(expected ivanmurzak|coplaydev)", + f"unity_teardown: unknown BMAD_AUTO_ENGINE_MCP={mcp!r} (expected ivanmurzak|coplaydev)", file=sys.stderr, ) rc = 2 diff --git a/tests/test_engine_plugin.py b/tests/test_engine_plugin.py index cc8091f..5d4d924 100644 --- a/tests/test_engine_plugin.py +++ b/tests/test_engine_plugin.py @@ -358,6 +358,52 @@ def test_unity_teardown_lingering_scan_no_false_match(tmp_path): assert mod._force_kill_lingering(tmp_path) == 0 +class _RecordingHost: + """A fake ProcessHost recording which pids it was asked to terminate/force-kill. + ``alive`` drives whether a pid is treated as a survivor past the grace window.""" + + def __init__(self, alive): + self._alive = alive + self.terminated: list[int] = [] + self.force_killed: list[int] = [] + + def terminate(self, pid): + self.terminated.append(pid) + + def force_kill(self, pid): + self.force_killed.append(pid) + + def is_alive(self, pid): + return self._alive + + +def test_unity_teardown_sweep_escalates_to_force_kill(tmp_path, monkeypatch): + """After de-dup the sweep routes through get_process_host(): a stubborn survivor + (is_alive stays True past the grace window) is SIGTERM'd then force-killed.""" + mod = _load_unity_teardown() + host = _RecordingHost(alive=True) + monkeypatch.setattr(mod, "get_process_host", lambda: host) + monkeypatch.setattr(mod, "_lingering_pids", lambda wt: [4242]) + monkeypatch.setattr(mod.time, "sleep", lambda _s: None) # don't really wait 10s + + assert mod._force_kill_lingering(tmp_path) == 1 + assert host.terminated == [4242] + assert host.force_killed == [4242] + + +def test_unity_teardown_sweep_skips_force_kill_when_terminate_suffices(tmp_path, monkeypatch): + """A pid that exits after the polite SIGTERM is never force-killed.""" + mod = _load_unity_teardown() + host = _RecordingHost(alive=False) # already gone by the first liveness poll + monkeypatch.setattr(mod, "get_process_host", lambda: host) + monkeypatch.setattr(mod, "_lingering_pids", lambda wt: [4242]) + monkeypatch.setattr(mod.time, "sleep", lambda _s: None) + + assert mod._force_kill_lingering(tmp_path) == 1 + assert host.terminated == [4242] + assert host.force_killed == [] + + def test_unity_ready_grace_explicit_override(monkeypatch): mod = _load_unity_ready() monkeypatch.setenv("BMAD_AUTO_ENGINE_EDITOR_MODE", "per_worktree") diff --git a/tests/test_portability_guard.py b/tests/test_portability_guard.py index a10651e..942a626 100644 --- a/tests/test_portability_guard.py +++ b/tests/test_portability_guard.py @@ -49,10 +49,10 @@ # `os.kill(pid, 0)` is a read-only existence probe on POSIX but *destructive* on # Windows (it maps to TerminateProcess). Confine it to the platform-guarded # liveness helpers, each on a line carrying a `# portability:` ack; everything -# else routes through the ProcessHost seam (`get_process_host().is_alive`). +# else routes through the ProcessHost seam (`get_process_host().is_alive`). The +# Unity teardown no longer probes directly — it delegates to the seam. KILL_PROBE_ALLOW = { "process_host.py", - "data/plugins/unity/unity_teardown.py", } # The two sanctioned `shell=True` spots: operator-authored command strings whose From 36305790e0a3e00e0544f04a5cf692065b119048 Mon Sep 17 00:00:00 2001 From: pbean Date: Sun, 28 Jun 2026 14:48:46 -0700 Subject: [PATCH 6/9] docs: document the platform-seam closeout (multiplexer registry + ProcessHost) Add docs/porting-to-a-new-os.md mapping the four OS seams (terminal multiplexer, ProcessHost lifecycle, hook interpreter, validate preflight), their registries and override env vars, and the end-to-end native-Windows port cost. Refresh the adapter- and plugin-authoring guides, ROADMAP, setup-guide, README, docs index, and FEATURES to the post-registry world: register_multiplexer/register_process_host instead of "return from get_multiplexer()", helper scripts running under sys.executable and using get_process_host() for pid lifecycle, and the BMAD_AUTO_MUX_BACKEND / BMAD_AUTO_PROCESS_HOST overrides. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 4 +- docs/FEATURES.md | 31 ++--- docs/README.md | 1 + docs/ROADMAP.md | 40 ++++-- docs/adapter-authoring-guide.md | 39 ++++-- docs/plugin-authoring-guide.md | 67 ++++++---- docs/porting-to-a-new-os.md | 225 ++++++++++++++++++++++++++++++++ docs/setup-guide.md | 7 +- 8 files changed, 345 insertions(+), 69 deletions(-) create mode 100644 docs/porting-to-a-new-os.md diff --git a/README.md b/README.md index cde2e2c..155e7f1 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ Inspired by the original [bmad-automator](https://github.com/bmad-code-org/bmad- ## Requirements - **Python 3.11+**, **tmux**, and a supported coding CLI — `claude` by default; `codex` and `gemini` via [profiles](#other-coding-clis). -- **Linux or macOS** (or **Windows via WSL**, which _is_ Linux — it runs as-is). tmux is the one terminal-multiplexer backend today, but it now sits behind a pluggable seam (`TerminalMultiplexer`), so a native-Windows backend can slot in later without touching the engine — see the [adapter authoring guide](docs/adapter-authoring-guide.md#two-axes-cli-vs-transport). Native Windows is not yet shipped. +- **Linux or macOS** (or **Windows via WSL**, which _is_ Linux — it runs as-is). tmux is the one terminal-multiplexer backend today, but it now sits behind a pluggable **registry** of OS seams (transport, process lifecycle, hook interpreter), so a native-Windows backend slots in as new files + a registration line each, with no engine edits — see [Porting bmad-auto to a new OS](docs/porting-to-a-new-os.md). Native Windows is not yet shipped. - A **BMAD v6 project** (`_bmad/bmm/config.yaml`, a `sprint-status.yaml` from `bmad-sprint-planning`) with the upstream `bmad-dev-auto` skill and the automator skill module from this repo installed (`bmad-auto-resolve`, `bmad-auto-sweep` — see [Installing the skill module](#installing-the-skill-module)). Standard BMAD skills stay untouched. ## Quick start @@ -62,7 +62,7 @@ bmad-auto tui # …or drive everything from the dashboard | Command | What it does | | -------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `bmad-auto init` | Install the bundled `bmad-auto-*` skills, the hook relay, `.automator/policy.toml`, and a runs-dir gitignore. `--cli ` (repeatable) targets specific agents; `--no-skills` / `--force-skills` control skill copying. | -| `bmad-auto validate` | Preflight every prerequisite: BMAD config, sprint-status, git, tmux, CLI binary, hook registration. | +| `bmad-auto validate` | Preflight every prerequisite: BMAD config, sprint-status, git, CLI binary, hook registration, and a platform check that reports the selected multiplexer's readiness and process host. | | `bmad-auto run` | Drive the dev → review → verify → commit loop. `--epic N`, `--story KEY`, `--max-stories N`, `--dry-run`. | | `bmad-auto sweep` | Triage + execute open `deferred-work.md` entries. `--no-prompt`, `--decisions-only`, `--max-bundles N`, `--repeat`, `--max-cycles N`, `--dry-run`. | | `bmad-auto resume ` | Continue a run paused at a gate, escalation, or interruption. | diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 1cb2174..f49011d 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -8,21 +8,21 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se ## Capability matrix (feature → problem addressed) -| Capability | What it does | Problem it addresses | -| ------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------- | -| Deterministic control loop | Story selection, retries, gates, completion checks run in plain Python | LLM-as-orchestrator is nondeterministic, hard to debug, and costs tokens for control flow | -| Trust-nothing verification | Checks on-disk artifacts (spec status, baseline-commit match, non-empty diff, sprint sync) + runs your test/lint commands before commit | Agents claim success without working code; broken builds slip through | -| Fresh-context adversarial review | Dev and review are separate sessions; review uses 2 parallel layers (Blind Hunter / Edge Case Hunter) | Self-review anchoring bias; implementer marks own work correct | -| Hook-based transport | Coding-agent hooks write structured event files; skills write `result.json` | Brittle terminal pane-scraping | -| Resumable state machine | Every run is on-disk state, resumable after gate/escalation/crash | Long unattended runs lost to interruptions | -| Plateau-defer | Stuck stories are skipped, stashed, and the run continues | One unconvergeable story blocking a whole sprint | -| Typed escalations + resolve workflow | CRITICAL pauses + notifies; interactive resolve agent re-arms the story | Ambiguous specs silently producing wrong code | -| Deferred-work sweeps | Triages an append-only ledger against real code, bundles + executes | Split-off goals and review findings get lost | -| Multi-CLI adapter + profiles | Generic driver runs claude/codex/gemini; per-stage overrides; TOML profiles; transport behind a pluggable multiplexer seam (tmux today) | Vendor lock-in; no way to mix models per stage; future non-tmux/Windows transport | -| Cost-weighted token budgets | Per-story budget counts cache reads at ~0.1x; raw totals displayed | Naive token caps misjudge real cost (cache reads dominate) | -| Non-invasive skill forks | Drives its own `bmad-auto-*` skill forks; reads `sprint-status.yaml` only | Modifying a user's standard BMAD install | -| Read-only TUI + launcher | Live dashboard over run-dir artifacts; launches detached runs | No visibility into what an unattended run is doing | -| Git worktree isolation (opt-in) | Each unit runs in its own worktree/branch (seeded with the adapters' gitignored MCP/CLI configs), merging back into the target locally; failed units kept for inspection | A long unattended run mutating the working tree you're actively using | +| Capability | What it does | Problem it addresses | +| ------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------- | +| Deterministic control loop | Story selection, retries, gates, completion checks run in plain Python | LLM-as-orchestrator is nondeterministic, hard to debug, and costs tokens for control flow | +| Trust-nothing verification | Checks on-disk artifacts (spec status, baseline-commit match, non-empty diff, sprint sync) + runs your test/lint commands before commit | Agents claim success without working code; broken builds slip through | +| Fresh-context adversarial review | Dev and review are separate sessions; review uses 2 parallel layers (Blind Hunter / Edge Case Hunter) | Self-review anchoring bias; implementer marks own work correct | +| Hook-based transport | Coding-agent hooks write structured event files; skills write `result.json` | Brittle terminal pane-scraping | +| Resumable state machine | Every run is on-disk state, resumable after gate/escalation/crash | Long unattended runs lost to interruptions | +| Plateau-defer | Stuck stories are skipped, stashed, and the run continues | One unconvergeable story blocking a whole sprint | +| Typed escalations + resolve workflow | CRITICAL pauses + notifies; interactive resolve agent re-arms the story | Ambiguous specs silently producing wrong code | +| Deferred-work sweeps | Triages an append-only ledger against real code, bundles + executes | Split-off goals and review findings get lost | +| Multi-CLI adapter + profiles | Generic driver runs claude/codex/gemini; per-stage overrides; TOML profiles; transport + process-lifecycle + hook-interpreter behind a pluggable OS-seam registry (tmux/POSIX today) | Vendor lock-in; no way to mix models per stage; future non-tmux/Windows transport | +| Cost-weighted token budgets | Per-story budget counts cache reads at ~0.1x; raw totals displayed | Naive token caps misjudge real cost (cache reads dominate) | +| Non-invasive skill forks | Drives its own `bmad-auto-*` skill forks; reads `sprint-status.yaml` only | Modifying a user's standard BMAD install | +| Read-only TUI + launcher | Live dashboard over run-dir artifacts; launches detached runs | No visibility into what an unattended run is doing | +| Git worktree isolation (opt-in) | Each unit runs in its own worktree/branch (seeded with the adapters' gitignored MCP/CLI configs), merging back into the target locally; failed units kept for inspection | A long unattended run mutating the working tree you're actively using | --- @@ -113,6 +113,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se ### Multi-CLI / multi-agent support - Generic adapter drives any CLI fitting the injection + hook-signal transport; CLI specifics live in declarative TOML profiles. Two independent axes: the **CLI** (`CodingCLIAdapter` + profile) and the **terminal transport** (`TerminalMultiplexer`) — tmux is the only backend today, behind a pluggable seam so a native-Windows backend can be added without touching the engine (see the [adapter authoring guide](adapter-authoring-guide.md#two-axes-cli-vs-transport)). +- The OS is abstracted by a **registry of seams**, each selecting an implementation by platform (with a test-override env var) and extended by a single registration line: the terminal multiplexer (`register_multiplexer`), the process-lifecycle `ProcessHost` (`register_process_host` — `terminate`/`force_kill`/`is_alive`/`identity`), and the hook interpreter (`ProcessHost.hook_interpreter()`); `bmad-auto validate` runs a platform preflight over them. Porting to a new OS is new files + registrations, no core edits — see [Porting bmad-auto to a new OS](porting-to-a-new-os.md). - Supported, E2E-verified: `claude` (reference), `codex` (≥ 0.139), `gemini` (≥ 0.46), `copilot` (GitHub Copilot CLI ≥ 2026-02 — the `copilot` binary, not the VS Code extension; `agentStop` turn-end, `-i` interactive launch, `--allow-all-tools`; pin a capable model — the free GPT-5 mini default is unreliable for multi-step skills). - Per-stage CLI/model overrides: run dev on one CLI/model, review on another (`[adapter.dev]`, `[adapter.review]`, `[adapter.triage]`). - Add a CLI without touching Python: drop a TOML profile in `.automator/profiles/.toml` (binary, prompt template, bypass flags, hook dialect, native→canonical event map). diff --git a/docs/README.md b/docs/README.md index 19b5e78..1c33d58 100644 --- a/docs/README.md +++ b/docs/README.md @@ -15,6 +15,7 @@ guides below go deeper, roughly in the order you'll need them. - **[Writing a bmad-auto plugin](plugin-authoring-guide.md)** — the plugin system: `plugin.toml` manifest, hooks, lifecycle stages, settings, the trust model, and workflow injection, with a worked walkthrough. - **[Writing a Game Engine plugin](game-engine-plugin-guide.md)** — the game-engine layer (built on the plugin system): driving a live engine Editor, the `editor_mode` ↔ `[scm] isolation` coupling, a minimal Godot example. - **[Writing a plugin for a specific Editor MCP](game-engine-mcp-guide.md)** — Editor-MCP specifics for the bundled Unity plugin: IvanMurzak vs CoplayDev, readiness probes, `per_worktree` isolation, and the full `BMAD_AUTO_*` env-var reference. +- **[Porting bmad-auto to a new OS](porting-to-a-new-os.md)** — the four OS seams (terminal multiplexer, process lifecycle, hook interpreter, validate preflight), their registries and override env vars, and what a native-Windows port costs end to end. - **[The Test Architect (TEA) plugin](tea-plugin-guide.md)** — the bundled `tea` plugin: installing TEA, the six advisory test-architecture steps it injects across runs and sweeps, the enable/blocking settings, and the escalate-on-gate behavior. ## Project direction diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 20082d0..fb6654d 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -9,25 +9,37 @@ Status legend: **planned** (agreed, not started) · **exploring** (shape still o ## Native Windows multiplexer backend -**Status:** planned · **Foundation:** multiplexer seam + portability hardening landed (v0.7.0) +**Status:** planned · **Foundation:** the full platform-seam series landed (multiplexer registry + `BaseTmuxBackend` + `ProcessHost` + hook interpreter + validate preflight, v0.7.6; original seam v0.7.0) The orchestrator no longer fuses tmux into the engine. All session/window/pane operations go through a single `TerminalMultiplexer` ABC (`src/automator/adapters/multiplexer.py`), -obtained from `get_multiplexer()`; `TmuxMultiplexer` (`adapters/tmux_backend.py`) is the -**only** file allowed to shell out to `tmux`, and it quarantines the POSIX `sh -c` -parked-window trailer. Alongside it, the scattered POSIX-isms were guarded behind a -platform-util seam (`terminate_pid`, detach kwargs, `SIGKILL` fallback, `os.devnull`) and -the Unity plugin's `/proc`/`/tmp`/`cp -a`/symlink primitives now degrade off Linux (with -`psutil` from the optional `windows` extra), all verified by a CI portability guard -(`tests/test_portability_guard.py`). **WSL already works today** — it _is_ Linux, so it -takes every fast path unchanged; this is purely about a future _native_ Windows host. +obtained from `get_multiplexer()`; the tmux backend — argv + spawn primitive in +`BaseTmuxBackend` (`adapters/tmux_base.py`), POSIX leaf `TmuxMultiplexer` +(`adapters/tmux_backend.py`) — is the **only** place allowed to shell out to `tmux`, and it +quarantines the POSIX `sh -c` parked-window trailer. Backends now self-**register** +(`register_multiplexer`, selected by platform with a `BMAD_AUTO_MUX_BACKEND` override) rather +than being hardcoded into `get_multiplexer()`. The process-lifecycle POSIX-isms moved behind a +matching seam, `ProcessHost` (`src/automator/process_host.py`): `terminate` / `force_kill` / +`is_alive` / `identity` (a PID-reuse guard) plus `hook_interpreter()` (so hook registration +never branches on platform), registered the same way (`register_process_host`, +`BMAD_AUTO_PROCESS_HOST`); `WindowsProcessHost` already ships. `bmad-auto validate` runs a +`_platform_preflight()` that reports the selected backend's readiness and names the process +host — so a new OS surfaces in preflight by registering, not by a `validate` edit. The Unity +plugin's `/proc`/`/tmp`/`cp -a`/symlink primitives degrade off Linux (with `psutil` from the +optional `non-linux` extra) and its pid lifecycle now delegates to `ProcessHost`; everything is +held by a CI portability guard (`tests/test_portability_guard.py`). **WSL already works today** +— it _is_ Linux, so it takes every fast path unchanged; this is purely about a future _native_ +Windows host. The remaining work is a real non-tmux backend (a "psmux"-style multiplexer) that implements -the `TerminalMultiplexer` contract on native Windows and is returned from `get_multiplexer()` -behind a platform/policy check. The seam is designed so this slots in with **no change to -the adapters, `runs.py`, `tui/launch.py`, `probe.py`, or `tui/data.py`** — a backend author -reads `multiplexer.py` for the contract and `tmux_backend.py` for the reference -implementation (see the [adapter authoring guide](adapter-authoring-guide.md#the-transport-contract-for-a-backend-author)). +the `TerminalMultiplexer` contract on native Windows and registers itself for `win32`. The +seams are designed so this slots in as **new files + one registration line each, with no +change to the adapters, `runs.py`, `tui/launch.py`, `probe.py`, `tui/data.py`, or +`cli.py`'s `validate`** (`WindowsProcessHost` and its hook interpreter are already in place +and registered). The end-to-end port path — both build options, the test-override env vars, +and exactly what a native-Windows port costs — is documented in +[Porting bmad-auto to a new OS](porting-to-a-new-os.md); the deep transport contract is in +the [adapter authoring guide](adapter-authoring-guide.md#the-transport-contract-for-a-backend-author). **Open questions:** what hosts the windows on native Windows (Windows Terminal panes, a ConPTY-based manager, a headless process supervisor?); how attach/detach and the parked diff --git a/docs/adapter-authoring-guide.md b/docs/adapter-authoring-guide.md index 3f2a46a..b9eddf7 100644 --- a/docs/adapter-authoring-guide.md +++ b/docs/adapter-authoring-guide.md @@ -26,13 +26,17 @@ These are independent and abstracted separately: - **Transport axis** — `TerminalMultiplexer` (`adapters/multiplexer.py`): how sessions, windows, and panes are created, observed, and torn down. The generic adapter never shells out itself — it goes through `self.mux`, obtained from - `get_multiplexer()`. The one backend today is tmux - (`adapters/tmux_backend.py`), which is the **only** file allowed to invoke - `tmux` (and the only place POSIX-shell trailers live). A future non-POSIX - backend (e.g. a native-Windows "psmux") implements the `TerminalMultiplexer` - contract and slots in behind `get_multiplexer()` with no change to the adapters. - A backend author reads `multiplexer.py` for the contract and `tmux_backend.py` - for the reference implementation. + `get_multiplexer()`. The one backend today is tmux: argv construction and the + single spawn primitive live in `BaseTmuxBackend` (`adapters/tmux_base.py`), with + the thin POSIX leaf `TmuxMultiplexer` (`adapters/tmux_backend.py`); together they + are the **only** files allowed to invoke `tmux` (and the only place POSIX-shell + trailers live). A future non-POSIX backend (e.g. a native-Windows "psmux") + registers itself via `register_multiplexer(...)` and slots in behind + `get_multiplexer()` with no change to the adapters. A backend author reads + `multiplexer.py` for the contract and `tmux_backend.py` / `tmux_base.py` for the + reference implementation. Transport is one of **four OS seams** — the others + (process lifecycle, hook interpreter, validate preflight) are mapped in + [Porting bmad-auto to a new OS](porting-to-a-new-os.md). ### The transport contract (for a backend author) @@ -40,12 +44,21 @@ Every part of the codebase that touches sessions, windows, or clients now goes through `get_multiplexer()` — not just the generic adapter but also `runs.py` (session listing/tagging, kill, attach argv), `tui/launch.py` (the control session and its parked orchestrator windows), `probe.py` (the throwaway probe -session), and `tui/data.py` (legacy-run liveness). A grep for `"tmux"` outside -`adapters/tmux_backend.py` should turn up only `shutil.which("tmux")` presence -checks, never an invocation. - -To add a backend, implement `TerminalMultiplexer` (`adapters/multiplexer.py`) and -return it from `get_multiplexer()`. The contract groups into: +session), and `tui/data.py` (legacy-run liveness). A grep for `"tmux"` outside the +tmux backend (`adapters/tmux_base.py` + `adapters/tmux_backend.py`) should turn up +only `shutil.which("tmux")` presence checks, never an invocation. + +To add a backend, build a `TerminalMultiplexer` (`adapters/multiplexer.py`) and +**register** it — `register_multiplexer(name, matches, factory)`, where +`matches(sys.platform)` decides automatic selection and `name` is the key the +`BMAD_AUTO_MUX_BACKEND` env var forces (for tests / overrides). `get_multiplexer()` +returns the first backend whose `matches` is true, with tmux as the default +fallback. There are two build paths: extend `BaseTmuxBackend` (`adapters/tmux_base.py`) +for a tmux-family backend — overriding only its single spawn primitive `_run()` +plus the few divergent methods (e.g. the parked-window trailer) — or implement +`TerminalMultiplexer` fresh for a host with no tmux-shaped CLI. The non-transport +seams of a full OS port are in +[Porting bmad-auto to a new OS](porting-to-a-new-os.md). The contract groups into: - **Sessions** — `has_session`, `new_session` (geometry is optional: agent sessions pin a fixed pane size because they are observed while detached; the diff --git a/docs/plugin-authoring-guide.md b/docs/plugin-authoring-guide.md index e9ba28e..fe44249 100644 --- a/docs/plugin-authoring-guide.md +++ b/docs/plugin-authoring-guide.md @@ -600,28 +600,51 @@ goes completely inert — proof of the trust gate. ## Platform portability -bmad-auto's core is portable Python and the tmux dependency is quarantined behind -the multiplexer seam (see the [adapter authoring guide](adapter-authoring-guide.md)). -Plugin **helper scripts** run as standalone `python3