feat(tui,adapters): warn when a reused tmux server would plant a stale state root (#731)

CHANGELOG.md

@@ -36,6 +36,11 @@ breaking changes may land in a minor release.
 
 ### Fixed
 
+- Warn once in the TUI when its tmux control session would hand new windows a
+  different state root than its own (a server started under another
+  `BMAD_LOOP_STATE_DIR`, `XDG_STATE_HOME` or `HOME`), naming both roots and the
+  `tmux set-environment` remedy, with its limits on a server shared by several
+  projects, instead of letting the run read as gone (#731).
 - Escalate an environment fault at the review-budget rescue gate instead of
   deferring the story as unconverged (DW-523).
 

docs/multiplexer-backends.md

@@ -58,6 +58,34 @@ parses and 3.2 lies a range that may run without complaint, and that nobody test
 psmux carries a separate version requirement of its own, for unrelated reasons — see below.
 The two floors are independent and neither implies the other.
 
+### A server started under another state root
+
+A tmux server is long-lived, and its panes inherit the environment the _server_ started
+with, not the environment of the client asking for the pane. A server started while
+`BMAD_LOOP_STATE_DIR` (or `XDG_STATE_HOME`, or `HOME`) named one state root, and reused
+later by a bmad-loop under another, runs every parked engine window under the first root, so
+a live run reads as gone (#731). When the TUI launches into its control session it asks tmux
+(`show-environment`) what a new window there would inherit, resolves the state root from
+that, and warns once — naming both roots — when it differs from its own. The launch still
+goes ahead; the warning detects, it does not fix. The remedy:
+
+- For new windows: `tmux set-environment -t =bmad-loop-ctl BMAD_LOOP_STATE_DIR <root>`. On
+  tmux, `bmad-loop-ctl` is one session shared by every bmad-loop project on that server, so this
+  re-roots new windows for all of them: use it only when no project there runs under another
+  state root. The session scope matters, because a session value overrides the global one; add
+  the same command with `-g` to cover new sessions as well. `tmux kill-server` also starts clean,
+  but it ends every session on the server, live runs and your own sessions included.
+- Shells already open there, window 0 included, keep the environment they started with. They
+  need `export BMAD_LOOP_STATE_DIR=<root>`, or recreating.
+
+Two projects that need different state roots cannot share one tmux server's control session
+safely until each launched run is handed its state root directly (#731).
+
+psmux cannot answer the question — its `show-environment` does not report inherited values
+— so there is no such warning on psmux. Its per-project registry is keyed on the state root,
+so a psmux launcher under one root never reaches a server started under another through
+the ordinary path.
+
 ## psmux (native Windows, experimental)
 
 On a native-Windows host the bundled **psmux** backend is the platform default. psmux is a

src/bmad_loop/adapters/multiplexer.py

@@ -38,6 +38,7 @@
 from collections.abc import Callable
 from dataclasses import dataclass
 from pathlib import Path
+from typing import Final, final
 
 from .. import envvars
 from .entrypoints import record_load_error
@@ -67,6 +68,24 @@ def parse_target(target: str) -> tuple[str, str | None] | None:
     return (session, window or None)
 
 
+@final
+class Unset:
+    """The type of :data:`UNSET`: a variable confirmed absent for new panes, as
+    answered by :meth:`TerminalMultiplexer.inherited_env`. Its own type rather
+    than ``""`` because absent and set-to-empty are different inputs to the
+    state-root cascade (an absent ``HOME`` takes the passwd fallback, an empty
+    one names ``/``). Compare with ``is UNSET``."""
+
+    __slots__ = ()
+
+    def __repr__(self) -> str:
+        return "UNSET"
+
+
+#: The one :class:`Unset` instance.
+UNSET: Final = Unset()
+
+
 class TerminalMultiplexer(ABC):
     """Transport backend for agent sessions: sessions, windows, and clients.
 
@@ -546,6 +565,32 @@ def legacy_registries(self) -> list[TerminalMultiplexer]:
         ``docs/porting-to-a-new-os.md``."""
         return []
 
+    def inherited_env(
+        self,
+        session: str,
+        name: str,
+        *,
+        on_fault: Callable[[str], None] | None = None,
+    ) -> str | Unset | None:
+        """What a new pane in ``session`` will inherit for the environment
+        variable ``name``: its value (``""`` included), :data:`UNSET` when it is
+        confirmed absent, or ``None`` when this transport cannot tell.
+
+        A multiplexer server is long-lived, and its panes inherit the
+        environment the server started with rather than the one of the client
+        asking for the pane, so a launcher cannot read the answer off its own
+        environment (#731). Must not raise. A query the backend supports but
+        could not complete (timeout, missing binary, an unexpected reply)
+        answers ``None`` **and** hands ``on_fault`` a one-line description, so a
+        caller can tell "tried and failed" from "cannot tell"; with no sink the
+        backend says nothing.
+
+        Non-abstract, defaulting to ``None`` with nothing reported, so released
+        out-of-tree backends keep working unchanged. psmux keeps this default:
+        its ``show-environment`` reports only values set through psmux itself,
+        never inherited ones."""
+        return None
+
     def window_pane_pids(self, target: str) -> list[int]:
         """Best-effort OS pids of ``target``'s pane root processes, for the kill
         escalation. Not abstract: backends that can't (or don't) report pids

src/bmad_loop/adapters/tmux_backend.py

@@ -6,7 +6,8 @@
 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, adding only
-the POSIX launch-pid prelude to each coding-CLI window (DW-507). See
+the POSIX launch-pid prelude to each coding-CLI window (DW-507) and the
+``inherited_env`` query psmux must not inherit (#731). See
 :mod:`.multiplexer` for the contract.
 
 ``subprocess`` and ``shutil`` are imported (and re-exported) here so existing
@@ -17,8 +18,10 @@
 from __future__ import annotations
 
 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
+import subprocess
+from collections.abc import Callable
 
+from .multiplexer import UNSET, Unset
 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
@@ -63,3 +66,51 @@ def _window_launch(self, env: dict[str, str], command: str) -> list[str]:
         """
         *env_args, command = super()._window_launch(env, command)
         return [*env_args, "/bin/sh", "-c", LAUNCH_PRELUDE, "sh", command]
+
+    def inherited_env(
+        self,
+        session: str,
+        name: str,
+        *,
+        on_fault: Callable[[str], None] | None = None,
+    ) -> str | Unset | None:
+        """The seam query (#731), answered by ``show-environment``: a new pane's
+        env is the global env overlaid with the session env, so the session
+        scope is asked first and the global scope only on a session miss.
+
+        Replies, measured on tmux 3.4: ``NAME=value`` is the value (``NAME=``
+        is a set-empty ``""``), ``-NAME`` is tmux's removal marker (known-unset),
+        and rc 1 with exactly ``unknown variable: NAME`` is a miss in that scope. Anything
+        else — another error such as ``no such session``, a timeout, a missing
+        binary, an unparseable reply — is a fault: ``None``, reported once
+        through ``on_fault``.
+
+        Here and not on :class:`~.tmux_base.BaseTmuxBackend`, which would hand
+        it to psmux: psmux's ``show-environment`` ignores the variable name and
+        hides inherited values, so its replies do not mean what this parse
+        reads them as."""
+        for scope in (["-t", f"={session}"], ["-g"]):
+            argv = ["show-environment", *scope, name]
+            try:
+                proc = self._run(argv, check=False)
+            except (subprocess.SubprocessError, OSError, UnicodeError) as exc:
+                return self._env_fault(argv, str(exc), on_fault)
+            reply = proc.stdout.removesuffix("\n")
+            if proc.returncode == 0:
+                if reply == f"-{name}":
+                    return UNSET
+                if reply.startswith(f"{name}=") and "\n" not in reply:
+                    return reply[len(name) + 1 :]
+                return self._env_fault(argv, f"unexpected reply {reply!r}", on_fault)
+            # A miss is exactly tmux's own line for exactly this name; anything
+            # merely containing the words (a socket path, say) is a fault.
+            if proc.returncode != 1 or proc.stderr.strip() != f"unknown variable: {name}":
+                detail = proc.stderr.strip() or f"exit {proc.returncode}"
+                return self._env_fault(argv, detail, on_fault)
+        return UNSET
+
+    def _env_fault(
+        self, argv: list[str], detail: str, on_fault: Callable[[str], None] | None
+    ) -> None:
+        if on_fault is not None:
+            on_fault(f"{self._BINARY} {' '.join(argv)} failed: {detail}")

src/bmad_loop/envvars.py

@@ -28,6 +28,7 @@
 
 import math
 import os
+from collections.abc import Mapping
 
 #: Overrides the per-session wall-clock budget, in seconds (test / E2E hook).
 SESSION_TIMEOUT_S = "BMAD_LOOP_SESSION_TIMEOUT_S"
@@ -85,8 +86,10 @@ def process_host() -> str | None:
     return os.environ.get(PROCESS_HOST)
 
 
-def state_dir() -> str | None:
-    """The overriding bmad-loop state root, or ``None`` when unset.
+def state_dir(env: Mapping[str, str] | None = None) -> str | None:
+    """The overriding bmad-loop state root, or ``None`` when unset. Read from
+    ``env`` when given (:func:`runs.resolve_state_root` asks on behalf of another
+    process's environment), else from this process's.
 
     Verbatim like the two name readers above — :func:`runs.state_root` uses the
     value as the state root itself, so an operator who names a directory gets that
@@ -108,4 +111,4 @@ def state_dir() -> str | None:
     *current directory*, so honouring it would silently root the control plane at
     whatever cwd the loop happened to be launched from.
     """
-    return os.environ.get(STATE_DIR) or None
+    return (os.environ if env is None else env).get(STATE_DIR) or None

src/bmad_loop/runs.py

@@ -527,7 +527,69 @@ def state_root() -> Path:
     silent — a control plane at the cwd, or at ``/``, that the *next* process to
     ask resolves somewhere else.
     """
-    override = envvars.state_dir()
+    return resolve_state_root(os.environ, passwd_home() if needs_passwd_home(os.environ) else None)
+
+
+def needs_passwd_home(env: Mapping[str, str]) -> bool:
+    """Whether :func:`resolve_state_root` over ``env`` reaches the passwd
+    fallback: the POSIX cascade with no override, no usable
+    ``XDG_STATE_HOME`` and no ``HOME`` — the one arm where
+    ``os.path.expanduser("~")`` consults the passwd database. Callers ask before
+    looking it up, so the lookup (an NSS query, possibly a network directory)
+    runs exactly where it did before the cascade was extracted, and nowhere
+    else."""
+    return (
+        sys.platform != "win32"
+        and not envvars.state_dir(env)
+        and _state_base(env.get("XDG_STATE_HOME")) is None
+        and "HOME" not in env
+    )
+
+
+def state_root_inputs() -> tuple[str, ...]:
+    """The environment variables :func:`resolve_state_root` reads on this
+    platform, in cascade order. A process that wants to know which root
+    *another* process would resolve (a multiplexer pane, #731) asks for exactly
+    these and nothing else."""
+    if sys.platform == "win32":
+        return (envvars.STATE_DIR, "LOCALAPPDATA", "USERPROFILE")
+    return (envvars.STATE_DIR, "XDG_STATE_HOME", "HOME")
+
+
+def passwd_home() -> str | None:
+    """The current user's passwd home directory, or ``None`` when there is no
+    passwd database (Windows) or no entry for this uid. It is the one input of
+    the POSIX cascade that is not in the environment: ``os.path.expanduser``
+    falls back to it when ``HOME`` is absent."""
+    if sys.platform == "win32":
+        return None
+    try:
+        import pwd
+    except ImportError:  # posixpath.expanduser's own guard: no passwd database
+        return None
+    try:
+        return pwd.getpwuid(os.getuid()).pw_dir
+    except KeyError:  # no entry for this uid (bpo-10496)
+        return None
+
+
+def resolve_state_root(env: Mapping[str, str], passwd_home: str | None) -> Path:
+    """The state root a process with environment ``env`` resolves: the
+    :func:`state_root` cascade over an explicit mapping, so the answer can be
+    computed for an environment other than this process's (a multiplexer pane
+    inheriting a server's env, #731). :func:`state_root` is this function over
+    ``os.environ``; the rules and their reasons are documented there.
+
+    ``passwd_home`` is the passwd entry's home directory, the one cascade input
+    that is not an environment variable, and ``None`` when there is no entry.
+    It is used only when ``HOME`` is **absent** from ``env``, which is
+    ``posixpath.expanduser``'s rule: a present ``HOME``, empty included, is
+    taken as given (an empty one folds to ``/``, which :func:`_state_base`
+    rejects), and an absent one with no passwd entry leaves ``~`` unexpanded,
+    which is relative and rejected the same way. The win32 arm never reads it.
+
+    Raises :class:`StateRootError` when no candidate answers."""
+    override = envvars.state_dir(env)
     if override:
         # `os.path.isabs` on the raw string, matching `_state_base` exactly rather
         # than `Path.is_absolute` — the rule and its reason are stated there.
@@ -541,17 +603,20 @@ def state_root() -> Path:
             )
         return Path(override)
     if sys.platform == "win32":
-        local = _state_base(os.environ.get("LOCALAPPDATA"))
+        local = _state_base(env.get("LOCALAPPDATA"))
         if local:
             return local / "bmad-loop" / "state"
-        profile = _state_base(os.environ.get("USERPROFILE"))
+        profile = _state_base(env.get("USERPROFILE"))
         if profile:
             return profile / "AppData" / "Local" / "bmad-loop" / "state"
     else:
-        xdg = _state_base(os.environ.get("XDG_STATE_HOME"))
+        xdg = _state_base(env.get("XDG_STATE_HOME"))
         if xdg:
             return xdg / "bmad-loop"
-        home = _state_base(os.path.expanduser("~"))
+        raw = env["HOME"] if "HOME" in env else passwd_home
+        # posixpath.expanduser's fold: trailing separators stripped, and an
+        # empty result is the root. No home at all leaves "~", which is relative.
+        home = _state_base("~" if raw is None else raw.rstrip("/") or "/")
         if home:
             return home / ".local" / "state" / "bmad-loop"
     raise StateRootError(
@@ -6788,7 +6853,7 @@ def rearm_event_notice(
                 f"the recorded spec for this story ({spec}) could not be re-opened to "
                 f"`{status}` — it is not a readable file from here{mount}, so the "
                 "re-drive reads that same path and finds no spec there to route on",
-                f"Restore the recorded spec path{_redrive_status_clause(entry)} before " "resuming",
+                f"Restore the recorded spec path{_redrive_status_clause(entry)} before resuming",
             )
         # No next_step, and deliberately: on this leg there is nothing to do to THIS
         # file. Whether anything is left to do at all is decided by the committed spec,

src/bmad_loop/tui/app.py

@@ -1944,5 +1944,12 @@ def run_tui(project: Path) -> int:
         mux_usable()
     except MultiplexerError:
         pass
-    BmadLoopApp(project).run()
+    app = BmadLoopApp(project)
+    # Launch warnings (the #731 state-root check) would print into the stderr
+    # Textual captures, so they toast instead — for the app's run only.
+    launch.warn_sink = lambda message: app.notify(message, severity="warning", markup=False)
+    try:
+        app.run()
+    finally:
+        launch.warn_sink = None
     return 0