diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ed0cc9aa..0c0d99e9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -263,7 +263,7 @@ jobs: # Moving this pin is a deliberate PR that also moves the floor and # retires the workarounds; psmux-canary.yml runs the same gate against # `latest` to say when that PR is due. - version: v3.3.8 + version: v3.4.0 github-token: ${{ github.token }} - name: Run the live psmux gate diff --git a/CHANGELOG.md b/CHANGELOG.md index 3400772a..d39b1808 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,9 +18,10 @@ breaking changes may land in a minor release. - Add the `plugins.trust` validate check: an ungranted, changed, enabled-but-ungranted, stale, missing or unreadable project plugin, or an unreadable grant store, is a problem (#496, #497). -- Add psmux live-gate tripwires for psmux#773 (pane `PATH` rebuilt from the registry), - psmux#775 (`show-environment NAME` ignores the name) and psmux#774 (a case-only - duplicate session is refused) (#879). +- Add psmux live-gate rows for psmux#774 (a case-only duplicate session is refused), + psmux#773 (a pane inherits the server's start `PATH`), psmux#583 (a `-`-leading option + value is stored) and psmux#775 (`show-environment NAME` answers for that one name) + (#879). - Add `bmad-loop resolve --abandon` (DW-535): close a replay run that escalated because its story was finished meanwhile. Uncommitted tracked and untracked changes (plus the run's own spec/board/ledger copies), or commits on a @@ -58,9 +59,24 @@ breaking changes may land in a minor release. guards links, anchors, tracker refs, block length and the catalog shape. - Name the per-command time limit in a timed-out verify command's failure reason; `--reverify` also names the key to raise. +- **Require psmux 3.4.0 or newer (#879).** `available()` refuses 3.3.8 and below, so **on + such a host the psmux backend reports unavailable and selection falls through: upgrade + psmux**. 3.3.8 silently unsets an option given a `-`-leading value (psmux/psmux#583). +- Name the psmux control session `bmad-loop-ctl` again, one per project registry, now that + psmux 3.4.0 keys its duplicate-server guard on the registry (psmux#599). A pre-upgrade + `bmad-loop-ctl-` session is still treated as a control session and never pruned, + but new launches, the TUI's attach and stop keys and the parked-window sweep use + `bmad-loop-ctl`; end the old session once its windows have parked (#733). + +### Removed + +- psmux: stop refusing an `@` option value that starts with `-`; psmux 3.4.0 stores it + verbatim (#879). ### Fixed +- Fire the TUI's stale state-root note on psmux too: the psmux backend now answers which + value a new pane inherits through psmux 3.4.0's named `show-environment` (#660). - Fix the OpenCode V2 SSE-reconnect end-to-end test failing on slow runners (Windows): the gap's HTTP probe completed the session before the reader reconnected. The fake now holds the turn busy until the reconnect is up. diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 16fc233c..bf3f8356 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -327,7 +327,7 @@ bmad-loop drives a BMAD sprint unattended: a deterministic Python loop launches - **Description:** On native Windows, the bundled psmux backend (a ConPTY tmux re-implementation driven through PowerShell) runs sessions without WSL. It is experimental. - **Properties:** - On a win32 host, psmux is the default backend whenever it reports itself available. - - psmux counts as available only when `psmux` and `pwsh` are both on PATH and psmux is 3.3.8 or newer. Otherwise selection moves on to the next backend. + - psmux counts as available only when `psmux` and `pwsh` are both on PATH and psmux is 3.4.0 or newer. Otherwise selection moves on to the next backend. - psmux refuses to open a window under PowerShell older than 7.3, or when the version cannot be read. This applies even when psmux is forced. - psmux sessions are registered in a per-project registry under the state root. To use an ambient `PSMUX_DATA_DIR` instead, set `[mux] honor_ambient_psmux_data_dir`. - **Config:** `[mux] backend`; `[mux] honor_ambient_psmux_data_dir`; `BMAD_LOOP_MUX_BACKEND=psmux` diff --git a/docs/multiplexer-backends.md b/docs/multiplexer-backends.md index a71f8e19..c952d110 100644 --- a/docs/multiplexer-backends.md +++ b/docs/multiplexer-backends.md @@ -60,9 +60,10 @@ with. A server started under one state root (`BMAD_LOOP_STATE_DIR`, `XDG_STATE_H notes once, naming both, when they differ. A shell that must agree can `export BMAD_LOOP_STATE_DIR=`. -psmux cannot report inherited values, so there is no such note on psmux. Its registry is -keyed on the state root, so a launcher under one root never reaches a server started under -another. +psmux (3.4.0 or newer) answers the same query, so the note fires on psmux too. psmux runs one +server per session, so the inherited environment is that session's own server's. Its +registry is keyed on the state root, so a launcher normally reaches only servers started +under its own root; the note covers the cases where it does not. ## psmux (native Windows, experimental) @@ -72,7 +73,7 @@ session and window model and the `bmad-loop-` session names. Requirements: -- **psmux 3.3.8 or newer** and **`pwsh`** on `PATH`. `available()` requires both binaries +- **psmux 3.4.0 or newer** and **`pwsh`** on `PATH`. `available()` requires both binaries and reads psmux's reported version; an older or unreadable version reports unavailable and selection falls through. - **PowerShell 7.3 or newer** as `pwsh`. The backend resolves `pwsh` to one absolute path, @@ -99,8 +100,8 @@ window `@3`. `psmux show-options -w` does not show these values; read one with session-qualified (`session:@N`). **Refused option values.** An `@` option value must read back verbatim. A value that is -empty, starts with `-`, contains a double quote or a line break, or has leading or trailing -whitespace is refused with a stderr warning, and the option reads as unset. Ordinary +empty, contains a double quote or a line break, or has leading or trailing whitespace is +refused with a stderr warning, and the option reads as unset. Ordinary Windows paths (spaced, UNC, apostrophed, trailing separator) pass. The project ownership tag is a hex digest of the project path, so it always passes. @@ -127,9 +128,13 @@ psmux ls `bmad-loop attach` needs none of this; it runs the client under the root it exported. -**The control session is per project on psmux:** `bmad-loop-ctl-<16-hex registry digest>`. -The TUI's toasts name the exact session. When no state root can be derived it is plain -`bmad-loop-ctl`, as on tmux. +**The control session is `bmad-loop-ctl`**, one per project registry on psmux. A session +named `bmad-loop-ctl-<16 hex>` is also a control session to bmad-loop, so `cleanup` never +prunes it. Nothing launches into it, though, so the TUI's attach and stop keys and the +parked-window sweep do not reach its windows. To end one, let its runs finish +(`bmad-loop stop` reaches a live engine wherever it runs) and check that every window shows +the exit banner (see `capture-pane` below). Then run +`psmux kill-session -t bmad-loop-ctl-<16 hex>`. **If the state root cannot be derived** (for example, `BMAD_LOOP_STATE_DIR` is a relative path), bmad-loop says so and leaves whatever `PSMUX_DATA_DIR` was in force. `cleanup` then @@ -246,9 +251,9 @@ safe once `capture-pane` has shown the banner. 2. `bmad-loop cleanup`, then run `delete` or `archive` again. 3. Or pass `--force`. It stops a live engine first, then removes the run without checking for a session, and leaves any orphaned session running. -A removal can still strand a session when the session listing misses it: the multiplexer binary is not on `PATH`, or on psmux a registry entry was briefly reaped. To clear a stranded session: +A removal can still strand a session when the session listing misses it, because the multiplexer binary is not on the removing process's `PATH`. To clear a stranded session: -1. Put the multiplexer back on `PATH`. On psmux after a reap, wait a few seconds for the server to republish the entry. +1. Put the multiplexer back on `PATH`. 2. Run `bmad-loop cleanup`. It prunes a session tagged to this project even when its run directory is gone. 3. For an untagged session, inspect it by hand. `bmad-loop attach` cannot reach it, because it needs the run directory. On psmux, paste the `$env:PSMUX_DATA_DIR = …` line `bmad-loop mux` prints first ([why](#where-psmux-sessions-live-the-per-project-registry)). Then attach with `psmux attach -t bmad-loop-` (`tmux attach -t …` on tmux). 4. Once you have confirmed from its contents that it is yours (the name does not carry the project), kill it: `psmux kill-session -t bmad-loop-` (or `tmux kill-session -t …`). diff --git a/docs/reference/run-state.md b/docs/reference/run-state.md index 939a0f33..15fd8723 100644 --- a/docs/reference/run-state.md +++ b/docs/reference/run-state.md @@ -106,7 +106,7 @@ A run id is `YYYYMMDD-HHMMSS-<4 hex>` in local time, for example `20261012-01152 | --------------- | --------------------------------------------------------------------------------------------------------------------------------------- | | agent session | `bmad-loop-`, one per run, tagged with the project key (`@bmad_project`); multiplexer-driven adapters only (opencode runs none) | | agent window | one per agent session, named after the task id (last 40 characters); its pane is piped to `logs/.log` | -| control session | `bmad-loop-ctl` on tmux; `bmad-loop-ctl-<16 hex>` on psmux (derived from the per-project registry) | +| control session | `bmad-loop-ctl`; on psmux one per project registry. A `bmad-loop-ctl-<16 hex>` session also counts as one, but nothing launches into it | | control window | `-`, where `kind` is `run`, `sweep`, `resume` or `resolve`; opened by the TUI, parks after the command exits | Teardown: diff --git a/docs/setup-guide.md b/docs/setup-guide.md index b5648c71..925b867d 100644 --- a/docs/setup-guide.md +++ b/docs/setup-guide.md @@ -44,7 +44,7 @@ see [Installing and upgrading](../README.md#installing-and-upgrading) in the REA - **Native Windows is experimental.** The bundled `psmux` backend (a ConPTY tmux re-implementation) drives runs there and is selected automatically as the win32 default when its prerequisites are present: the `psmux` and `pwsh` (PowerShell) binaries on - `PATH`, with `psmux` 3.3.8 or newer (older releases read as unavailable — see + `PATH`, with `psmux` 3.4.0 or newer (older releases read as unavailable — see [multiplexer backends](multiplexer-backends.md#psmux-native-windows-experimental) for why). - It is below the Linux/macOS/WSL support tier; open native-Windows work is in [the roadmap](ROADMAP.md#native-windows-multiplexer-backend), and the port path is in diff --git a/docs/testing.md b/docs/testing.md index f89faed5..befaa318 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -206,7 +206,7 @@ the live/E2E gates; in practice it holds everywhere, and the mechanism differs p | `test_stories_e2e.py` | tmux + the real `bmad-loop run/resolve/resume` CLI | Scripted fake `claude` variants (bash string constants) wired in as a custom TOML profile; the fake writes its own SessionStart/Stop hook events | CI Linux job (the fakes use GNU coreutils + `setsid`) | | `test_opencode_live.py` | A real `opencode serve` HTTP server | **Never sends a prompt**: only spawn/teardown and the health/OpenAPI/session/event endpoints of the detected major (1.x or 2.x); the prompt endpoint is checked against the OpenAPI schema, never called | Manual: a POSIX box with a runnable `opencode` | | `test_opencode_live_turn.py` | A real OpenCode 2.x server running one full turn | **Prompts reach only a local fake**: the one test that prompts sends every LLM request to an in-process stdlib OpenAI-compatible server on 127.0.0.1, under a scrubbed env (scratch HOME/XDG, no credentials), a mock-only provider config and a deny on the hosted provider, and asserts the fake saw every request | Manual: a POSIX box with a runnable OpenCode 2.x `opencode` | -| `test_psmux_live.py` | A real psmux on Windows | **Parked windows only**: every window runs `pwsh -NoProfile -Command exit 0`. `test_premise_*` probes are inverted (red means a workaround became droppable); `test_adopted_*` probes are ordinary | CI Windows job + any Windows dev box with psmux 3.3.8+ and `pwsh` | +| `test_psmux_live.py` | A real psmux on Windows | **Parked windows only**: every window runs `pwsh -NoProfile -Command exit 0`. `test_premise_*` probes are inverted (red means a workaround became droppable); `test_adopted_*` probes are ordinary | CI Windows job + any Windows dev box with psmux 3.4.0+ and `pwsh` | Never "fix" a gate to call a real CLI, and never add a completion path that trusts LLM output: sessions complete only on hook Stop events or window death, and the fakes exercise exactly diff --git a/docs/tui-guide.md b/docs/tui-guide.md index 9873d563..ae09f2b9 100644 --- a/docs/tui-guide.md +++ b/docs/tui-guide.md @@ -53,8 +53,7 @@ validate viewers) are the tightest thing the TUI draws: The TUI never runs an engine in-process. - **Launcher.** `r`, `s`, `e`, `R` and `V` spawn detached `bmad-loop` processes - as windows of a dedicated control session, `bmad-loop-ctl` (on psmux the name - carries a per-project suffix; the launch toast names the exact session). + as windows of a dedicated control session, `bmad-loop-ctl`. Windows are named `run-`, `sweep-`, `resume-` and so on, and run the same Python interpreter as the TUI. A window stays open after its command exits, showing `[bmad-loop exited — press enter]`, so you diff --git a/src/bmad_loop/adapters/multiplexer.py b/src/bmad_loop/adapters/multiplexer.py index 006d0dae..b472d0c1 100644 --- a/src/bmad_loop/adapters/multiplexer.py +++ b/src/bmad_loop/adapters/multiplexer.py @@ -631,9 +631,8 @@ def inherited_env( 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.""" + out-of-tree backends keep working unchanged. tmux and psmux answer it + through ``show-environment``.""" return None def window_pane_pids(self, target: str) -> list[int]: diff --git a/src/bmad_loop/adapters/psmux_backend.py b/src/bmad_loop/adapters/psmux_backend.py index d7dd0374..f955c582 100644 --- a/src/bmad_loop/adapters/psmux_backend.py +++ b/src/bmad_loop/adapters/psmux_backend.py @@ -64,9 +64,10 @@ rather than inventing one — and the collision it needs is an independently created session sharing a ``bmad-loop-`` name: an operator's, or another run's on a colliding id (see #531). The psmux -behaviors cited in this module were read from the psmux source at tag -``v3.3.8`` — the oldest build ``available()`` admits — and the safe -observable subset is probed in ``tests/test_psmux_live.py``. +behaviors cited in this module were read from the psmux source at the tag +each citation names, most at ``v3.3.8``; ``available()`` admits 3.4.0 and +later, and the safe observable subset is probed live on the installed build in +``tests/test_psmux_live.py``. See :mod:`.multiplexer` for the contract. """ @@ -79,9 +80,10 @@ import shutil import subprocess import sys -from collections.abc import Mapping +from collections.abc import Callable, Mapping from pathlib import Path +from .multiplexer import Unset from .tmux_base import PARKED_RETURN_DETACH, TMUX_TIMEOUT_S, BaseTmuxBackend, TmuxError _ENV_NAME = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") @@ -562,10 +564,15 @@ def _parked_trailer(self, return_opt: str) -> str: # # Every psmux call is a call-operator line on the absolute path the # backend resolves (`_psmux_path`), because a bare name resolves on the - # pane's PATH, which psmux builds from the registry rather than from the - # server's environment (psmux/psmux#773): another build, or none at all, - # with every error going to $null (#877). The path is baked in when the - # window is minted, so a window parked before an upgrade keeps its own. + # pane's PATH: another build, or none at all, with every error going to + # $null (#877). The admitted builds give a pane the PATH of the process + # that started its server (psmux/psmux#773), but that need not be this + # launcher's: a long-lived control session can be served by a server + # started earlier, from another PATH or by a pre-floor psmux still + # running after an upgrade (which rebuilds PATH from the registry), and + # a relative PATH entry stops resolving once the pane's working + # directory differs. The path is baked in when the window is minted, so + # a window parked before an upgrade keeps its own. mux = "& " + _pwsh_quote(self._psmux_path()) probe = ( '"$(' + mux + " display-message -p -t $env:TMUX_PANE '#{window_id}' 2>$null)\".Trim()" @@ -939,10 +946,8 @@ def _transportable(value: str) -> bool: # break, and leading/trailing whitespace stay refused however cleanly # the wire now carries them — a value that reads back different is the # same silently-unprunable window whichever hop mangled it. An empty - # value is a silent server-side no-op (`-u` is the verb for that), and a - # `-`-leading one is still dropped as a flag server-side, or flips the - # key to unset at rc 0 (psmux/psmux#583, open upstream). - if not value or value.startswith("-"): + # value is a silent server-side no-op (`-u` is the verb for that). + if not value: return False if '"' in value or value != value.strip(): return False @@ -1574,15 +1579,39 @@ def pipe_pane(self, window_id: str, log_file: Path) -> None: file=sys.stderr, ) + def inherited_env( + self, + session: str, + name: str, + *, + on_fault: Callable[[str], None] | None = None, + ) -> str | Unset | None: + """The seam query (#731), with tmux's replies since psmux/psmux#775 + (``src/environ.rs``, source-read at v3.4.0): the session scope first, + then the global one, through the base's ``_show_environment``. Two + psmux specifics, both measured on 3.4.0. ``-g`` is session-targeted: + one server per session makes "global" that session's server start + environment, and an untargeted ``-g`` answers from whichever server it + lands on. The echoed name is matched case-insensitively: the reply + spells the name as the environment block stores it (``PATH`` can come + back ``Path=``). + + Ceiling: under ``PSMUX_BARE_ENV`` a pane gets an allowlist, not what + this answers (see ``_warn_if_bare_env``); bmad-loop does not support + that mode and already warns about it.""" + target = ["-t", f"={session}"] + return self._show_environment([target, ["-g", *target]], name, on_fault, fold_case=True) + # Releases up to this version are refused. 3.3.6 and older force-kill a # recycled PID during pane teardown and let orphaned servers accumulate — - # engine-fatal on its own. 3.3.7 is excluded for a second reason: 3.3.8 is - # the build this backend is written against, and several verbs here now - # assume its fixes rather than routing around the defects (a direct - # `pipe-pane -o` flag transport, a `select-window` id target, the `=name` - # kill-session form, and the control-line shapes `_transportable` admits). - # On 3.3.7 those would fail silently, so the floor forbids it outright. - _LAST_UNSUPPORTED = (3, 3, 7) + # engine-fatal on its own. 3.3.7 lacks the fixes several verbs here assume + # rather than route around (a direct `pipe-pane -o` flag transport, a + # `select-window` id target, the `=name` kill-session form, and the + # control-line shapes `_transportable` admits). 3.3.8 stores a `-`-leading + # option value as the unset flag at rc 0 (psmux/psmux#583), a value + # `_transportable` no longer refuses. On those builds the failures are + # silent, so the floor forbids them outright. + _LAST_UNSUPPORTED = (3, 3, 8) # Class-level default; instances shadow it on first probe. Never assign on # the class outside tests — that would poison every future instance. _version_ok: bool | None = None diff --git a/src/bmad_loop/adapters/tmux_backend.py b/src/bmad_loop/adapters/tmux_backend.py index 20ef7229..14e908c9 100644 --- a/src/bmad_loop/adapters/tmux_backend.py +++ b/src/bmad_loop/adapters/tmux_backend.py @@ -7,7 +7,7 @@ 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) and the -``inherited_env`` query psmux must not inherit (#731). See +scopes of its ``inherited_env`` query (#731). See :mod:`.multiplexer` for the contract. ``subprocess`` and ``shutil`` are imported (and re-exported) here so existing @@ -18,10 +18,10 @@ from __future__ import annotations import shutil # noqa: F401 — re-exported for callers/tests reaching the spawn seam -import subprocess +import subprocess # noqa: F401 — re-exported for callers/tests reaching the spawn seam from collections.abc import Callable -from .multiplexer import UNSET, Unset +from .multiplexer import 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 @@ -84,41 +84,7 @@ def inherited_env( ) -> 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}") + scope is asked first and the global scope only on a session miss. The + parse is the base's ``_show_environment``; the scopes are this leaf's, + since tmux's ``-g`` is server-wide and psmux's is not.""" + return self._show_environment([["-t", f"={session}"], ["-g"]], name, on_fault) diff --git a/src/bmad_loop/adapters/tmux_base.py b/src/bmad_loop/adapters/tmux_base.py index d22b5210..3e97f3b9 100644 --- a/src/bmad_loop/adapters/tmux_base.py +++ b/src/bmad_loop/adapters/tmux_base.py @@ -37,7 +37,14 @@ from collections.abc import Callable from pathlib import Path -from .multiplexer import PARKED_BANNER, MultiplexerError, TerminalMultiplexer, fold_version +from .multiplexer import ( + PARKED_BANNER, + UNSET, + MultiplexerError, + TerminalMultiplexer, + Unset, + fold_version, +) TMUX_TIMEOUT_S = 30 # Per-window option value (vs a pane target) telling the parked trailer to detach @@ -151,6 +158,60 @@ def _run( raise TmuxError(f"{self._BINARY} {' '.join(argv[:2])} failed: {proc.stderr.strip()}") return proc + def _show_environment( + self, + scopes: list[list[str]], + name: str, + on_fault: Callable[[str], None] | None, + *, + fold_case: bool = False, + ) -> str | Unset | None: + """The ``inherited_env`` parse (#731), shared by the leaves that answer + it: ``show-environment NAME`` is asked of each scope in order, + and the first answer wins. Private, so a tmux-family leaf keeps the + seam's Unknown default until it names the scopes its binary answers. + + Replies, measured on tmux 3.4 and psmux 3.4.0: ``NAME=value`` is the + value (``NAME=`` is a set-empty ``""``), ``-NAME`` is the removal marker + (known-unset), and rc 1 with exactly ``unknown variable: NAME`` is a miss + in that scope, so the next one is asked; a miss in every scope is + known-unset. 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``. ``fold_case`` matches the + echoed name case-insensitively, for a binary that answers with the + spelling it stores (psmux on Windows: ``PATH`` comes back ``Path=``).""" + + def same(echoed: str) -> bool: + return echoed.casefold() == name.casefold() if fold_case else echoed == name + + for scope in scopes: + argv = ["show-environment", *scope, name] + try: + proc = self._run(argv, check=False) + except (subprocess.SubprocessError, OSError, UnicodeError, TmuxError) as exc: + return self._env_fault(argv, str(exc), on_fault) + reply = proc.stdout.removesuffix("\n") + if proc.returncode == 0: + if "\n" not in reply: + if reply.startswith("-") and same(reply[1:]): + return UNSET + echoed, eq, value = reply.partition("=") + if eq and same(echoed): + return value + return self._env_fault(argv, f"unexpected reply {reply!r}", on_fault) + # A miss is exactly the binary'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}") + def _session_proved_gone(self, proc: subprocess.CompletedProcess[str]) -> bool: """Whether a non-zero listing exit PROVES the session no longer exists. diff --git a/src/bmad_loop/runs.py b/src/bmad_loop/runs.py index 60041956..4de83aba 100644 --- a/src/bmad_loop/runs.py +++ b/src/bmad_loop/runs.py @@ -253,8 +253,9 @@ def is_reserved_run_id(value: str) -> bool: def run_id_aliases_control_session(value: str) -> bool: """True when ``session_name(value)`` names a session that can BE a live - control session: the fixed name (id ``ctl``) or a per-registry digest name - (id ``ctl-<16 hex>`` — the only suffix :func:`ctl_session_for` can mint). + control session: the fixed name (id ``ctl``) or the per-registry digest + name earlier releases minted on psmux (id ``ctl-<16 hex>``; see + :func:`is_ctl_session_name`). The adapter's ensure-session would *adopt* that live session as the run's own, and the run's teardown would kill the whole control session, every parked window of every run in it — so the project-free READ paths key on @@ -264,7 +265,7 @@ def run_id_aliases_control_session(value: str) -> bool: this name be a control session's on some registry" — and it must stay out of any site asking the *instance* question ("is it the control session this process addresses"): :func:`live_session_may_be_ours` - compares against the actual names (the fixed one plus this project's + compares against the actual name (this project's :func:`ctl_session_for`), because discounting the whole shape there destroyed run dirs under live `ctl-` agents on tmux. @@ -1802,93 +1803,39 @@ def kill_session(run_id: str, mux: TerminalMultiplexer | None = None) -> bool: def ctl_session_for(project: Path, mux: TerminalMultiplexer | None = None) -> str: - """The control-session name this project's launches and lookups share. - - On a transport with no registry namespace (tmux) it is the fixed - :data:`CTL_SESSION`, machine-shared as it has always been. On a namespacing - transport (psmux) the name carries the registry's identity — a 16-hex - digest of the derived registry root — because the two scopes genuinely - differ: the session lives *per registry*, but psmux's duplicate-server - guard is a mutex keyed on the session name alone, across every registry - in the **login session** (``Local\\psmux-session-{name}`` over - ``port_file_base()`` — the ``Local\\`` kernel-object namespace is - per-login-session, not machine-global; ``server/mod.rs:853`` / - ``platform.rs:346`` / ``types.rs:1345``, source-read at v3.3.8 — - ``PSMUX_DATA_DIR`` never enters it). A fixed name therefore admits ONE - control session across every registry a desktop session can reach, - and the second project's create is rejected as a duplicate server — its - TUI launch fails instead of minting its own session (measured: a second - registry answers ``new-session`` rc 1 for the fixed name while the first - registry's server lives, and rc 0 for a per-registry name). - - The digest is over ``mux_registry_root(project)`` **resolved**: the name - must be unique per *physical* registry, and the resolved path is that - registry's identity — (project, state root), both axes; ``project_tag`` - alone would recreate the collision for one project under two state roots. - Resolved rather than as spelled because two spellings of one state root - (``C:\\work\\state`` vs ``C:\\work\\alias\\..\\state``) reach **one** - registry — Windows resolves both to the same files, and psmux keeps the - spelling only while constructing those paths (``src/paths.rs:79``, - source-read at v3.3.8; convergence measured) — so an as-spelled digest - minted two control sessions inside one registry, each blind to the other's - parked windows: the split-control-plane failure again, one level up. Same - rule ``project_tag`` already states: resolve *before* digesting. - - …and then ``os.path.normcase``, because ``resolve()`` can only return the - filesystem's stored case for a path that **exists**, and the registry - root usually does not yet exist at the moment the name is needed (psmux - ``create_dir_all``\\s it at first spawn). Two case spellings of a - not-yet-created state root resolve to two strings, digest to two names — - and then land in ONE physical registry, because NTFS folds case when - psmux opens the ``.port`` files (measured). ``normcase`` folds exactly - where the filesystem does: it lowercases on Windows and is the identity - on POSIX, where case is significant and two case spellings ARE two - registries — folding there would merge genuinely distinct roots. - Ceiling, named: ``normcase`` lowercases with ``str.lower``, which can - disagree with NTFS's own fold table for a few non-ASCII case pairs; a - state root spelled in two such casings of the same non-ASCII name stays - split, as it is for every other digest of an operator-supplied path. - - When the operator's own root is honoured (``[mux] - honor_ambient_psmux_data_dir``), that settled root is digested alongside - the derived one: the name stays per project, and changes with the - registry it lives in. - - The degrade arm (namespaced transport, underivable state root) answers - the fixed name: that arm runs on the transport's shared default registry, - where a shared session scoped by per-window project tags is the correct, - tmux-shaped semantic — and where a pre-#537 legacy ctl session under the - fixed name may exist to be reused rather than collided with. - """ - mux = mux or get_multiplexer() - if not mux.has_registry_namespace(): - return CTL_SESSION - try: - derived = mux_registry_root(project) - scope = os.path.normcase(str(derived.resolve())) - # An honoured operator root (#729) is a different physical registry, so - # it gets a different name — or turning the opt-in on would mint the - # same name in the new registry while the old one's control session - # still holds psmux's mutex. Not honoured, the export settled the - # derived root and the name is byte-identical to before. Compared as - # identities, resolved and case-folded like `scope`, so another spelling - # of the derived registry is still that registry. The root is the - # backend's, not the environment's, so a bound instance answers for its - # own registry. - settled = mux.registry_root() - if settled and Path(settled).is_absolute(): - pinned = os.path.normcase(str(Path(settled).resolve())) - if pinned != scope: - scope += "\0" + pinned - except (StateRootError, OSError, RuntimeError): - return CTL_SESSION - return f"{CTL_SESSION}-{hashlib.sha256(os.fsencode(scope)).hexdigest()[:16]}" + """The control-session name this project's launches and lookups share: the + fixed :data:`CTL_SESSION`, on every bundled transport. + + On tmux one server serves the machine and the session really is + machine-wide, scoped by the per-window project tag. On psmux it lives in + the project's own registry, so each project holds its own session under the + one fixed name: psmux keys its one-server-per-name guard on the data root + as well as the name (psmux/psmux#599, ``session_mutex_name`` over + ``data_root_tag()``, source-read at v3.4.0), so two registries can each hold + a ``bmad-loop-ctl``. Earlier releases minted ``bmad-loop-ctl-<16 hex>`` on + psmux to dodge a guard keyed on the name alone; see + :func:`is_ctl_session_name` for what still recognises that shape. + + The mint and instance side of the control-session question, kept apart + from the shape side (:func:`is_ctl_session_name`) on purpose: every site + that *addresses* this project's control session resolves the name here, and + every reader that asks whether some name *is a* control session asks the + shape predicate, which answers for names this release never mints.""" + return CTL_SESSION def is_ctl_session_name(name: str) -> bool: """Whether ``name`` is a control session's name — the fixed - :data:`CTL_SESSION`, or ``bmad-loop-ctl-<16 hex>``, the ONE suffix shape - :func:`ctl_session_for` can mint. + :data:`CTL_SESSION`, or ``bmad-loop-ctl-<16 hex>``, the per-registry shape + earlier releases minted on psmux. + + The digest shape is still recognised, though :func:`ctl_session_for` no + longer mints it, because a control session started before an upgrade keeps + running under it with live runs in its windows. Recognised, it stays out + of the prune and of every kill path, and ``in_ctl_session`` still answers + True inside it; it is not adopted — launches, attach, stop and the + parked-window sweep address :func:`ctl_session_for` only — so it ends when + the operator ends it. The shape predicate exists because several readers ask "is this A control session" without a project in hand: the agent-session parser must exclude @@ -1898,7 +1845,7 @@ def is_ctl_session_name(name: str) -> bool: and ``in_ctl_session`` classifies whatever session this process woke up inside. - Exactly the mintable shapes, no wider: an arbitrary suffix + Exactly the shapes a release has minted, no wider: an arbitrary suffix (``bmad-loop-ctl-foo``) is NOT a control session — it is the agent session of a run an older release accepted as ``--run-id ctl-foo``, and reading it as a control session made it unreachable by ``stop`` and the @@ -3150,85 +3097,53 @@ def live_session_may_be_ours( A stronger shape was built and withdrawn: a proof discipline (block unless the transport *proves* the session absent) fell to four consecutive reviews, each refuting its newest proof source — the transports genuinely offer none. - psmux's registry is advisory and self-healing (its server re-creates a - reaped port file on a 5 s tick, source-read at v3.3.8), a binary's PATH - presence is per-process while the server is not, and the listing is - load-sensitive; so a "proof of absence" either wedges every removal behind - `--force` or quietly accepts a refutable proof. + psmux's registry is advisory and self-healing (its server re-writes its own + entry on a 5 s tick), a binary's PATH presence is per-process while the + server is not, and the listing is load-sensitive; so a "proof of absence" + either wedges every removal behind `--force` or quietly accepts a refutable + proof. Revisited with the cost measured (#732), and the degrade is KEPT — not for - want of a stronger read, but because the more dangerous of the two costs - reaches this function as an ordinary answer. On psmux 3.3.8 a live session's - registry entry is reaped by any `has-session` whose 500 ms connect does not - land, and until the server's registry maintenance re-writes it the listing - does not name that session while reporting no fault at all: exit **0**, no - stderr (measured; the listing empty in that run because the session was the - only one, the server pid alive throughout, the entry back 1.7 s later). - That maintenance runs when the server's own loop notices 5 s have elapsed — - a nominal interval, not a scheduling guarantee, so 1.7 s is a sample and 5 s - is not a bound. The listing therefore does not fail, it answers wrongly — - and the read this guard makes is the one with no discrimination in it at - all. ``mux_sessions()`` lands on ``BaseTmuxBackend.list_sessions``, which - returns ``[]`` under the three conditions its own comment names — the binary - is missing, no server is running, or the query itself fails — and returns - nothing to say which. None of the three raises: the bundled backend folds - ``SubprocessError`` and ``OSError`` into that same sentinel, and only an - out-of-tree backend raises ``MultiplexerError`` here. A failed query does - now say so (DW-458, ``_warn_unproven_listing`` — on stderr, or through - ``warn`` below), but a report is not a signal this function conditions on - — and the reap reaches none of - those branches anyway, because its exit is 0. So it leaves neither a - signal nor a word on stderr. - - The second cost lasts as long as its cause: a process whose PATH lacks the - binary reads every session as absent for as long as that PATH does (measured - through the seam — the sessions reappear when PATH is restored), because the - binary is per-process while the server is not. That one is no mystery to the - transport: ``list_sessions`` decides it from ``shutil.which`` on purpose, - being one of the two methods that report what exists, and folds it into the - same ``[]`` deliberately (its own comment: the absence of sessions and the - absence of the multiplexer are indistinguishable here). The fold is the - seam's decision; what reaches this function is its result. - - What the two costs share is not a shape but a verdict. A reap takes one - session's entry, so its listing may still name every other session on the - box; a missing binary yields nothing at all. Both arrive as the same - *negative membership* — this run's name is not in the listing — which is - exactly what a genuinely dead session produces, and there is no fourth - answer to distinguish them by. - - The named alternatives were weighed and refused. Retry-with-delay, in the - shape this guard could hold, pays a delay on every removal that finds no - session — the common case — once per run inside `clean`; a retry hoisted to - command scope would pay it once, but neither can bound a window whose close - is a nominal tick rather than a deadline. A tag-first read draws on the same - listing and is blind in exactly the same window. Warning whenever the - multiplexer is unusable fires on every box that simply has no multiplexer, - which is the noise ``BaseTmuxBackend._warn_unproven_listing`` already stays - silent about. What is left is the accepted ceiling, written down here and in - docs/multiplexer-backends.md so an operator meets it as a documented limit rather than a - surprise. - - The psmux half is a *defect*, not a design: four sibling reap sites in that - binary gate on `ConnectionRefused` precisely because "a timeout means - busy-but-alive and must not be deleted" (its own comment), the `has-session` - site does not, and tmux 3.4 blocks rather than reaps under the same - treatment (measured). Reported as psmux/psmux#622, which is the thing to - watch. Retiring it takes two parties, though, and an upstream fix alone is - not enough: the reap window closes for this project once a psmux release - repairs that site AND the supported floor is raised to that release or - later, since an installed 3.3.8 keeps reaping whatever master does. And it - retires only itself — the PATH case is ours, and so are the two arms above - it, a backend that cannot be selected and a listing that raises. What that - fix removes is one measured case, not the ceiling. + want of a stronger read, but because the cost reaches this function as an + ordinary answer. ``mux_sessions()`` lands on + ``BaseTmuxBackend.list_sessions``, which returns ``[]`` under the three + conditions its own comment names — the binary is missing, no server is + running, or the query itself fails — and returns nothing to say which. None + of the three raises: the bundled backend folds ``SubprocessError`` and + ``OSError`` into that same sentinel, and only an out-of-tree backend raises + ``MultiplexerError`` here. A failed query does now say so (DW-458, + ``_warn_unproven_listing`` — on stderr, or through ``warn`` below), but a + report is not a signal this function conditions on. + + The ceiling is the PATH case, and it lasts as long as its cause: a process + whose PATH lacks the binary reads every session as absent for as long as + that PATH does (measured through the seam — the sessions reappear when PATH + is restored), because the binary is per-process while the server is not. + That one is no mystery to the transport: ``list_sessions`` decides it from + ``shutil.which`` on purpose, being one of the two methods that report what + exists, and folds it into the same ``[]`` deliberately (its own comment: + the absence of sessions and the absence of the multiplexer are + indistinguishable here). The fold is the seam's decision; what reaches this + function is its result — a *negative membership*, this run's name not in + the listing, which is exactly what a genuinely dead session produces. + + The named alternatives were weighed and refused. A retry cannot help: the + PATH is this process's own and does not change between tries. A tag-first + read draws on the same listing and is blind in exactly the same way. + Warning whenever the multiplexer is unusable fires on every box that simply + has no multiplexer, which is the noise + ``BaseTmuxBackend._warn_unproven_listing`` already stays silent about. What + is left is the accepted ceiling, written down here and in + docs/multiplexer-backends.md so an operator meets it as a documented limit + rather than a surprise. A backend that cannot be selected and a listing + that raises meet it too, through the two signalled arms below. Two registry-root-era additions on that unchanged contract: - **The control-alias discount.** An id whose session name is one of THE - control session's own names — the fixed :data:`CTL_SESSION`, or this - project's :func:`ctl_session_for` — answers False before any transport - read: that session is the control plane's, never claimed through a run - dir, so its liveness is not evidence about the run, and blocking removal + **The control-alias discount.** An id whose session name is THE control + session's own name — this project's :func:`ctl_session_for` — answers + False before any transport read: that session is the control plane's, + never claimed through a run dir, so its liveness is not evidence about the run, and blocking removal on it wedged exactly the recovery (`bmad-loop delete ctl`) the resume refusal points an operator at, for as long as the machine had a control session at all. This is the *instance* question, deliberately not @@ -3236,9 +3151,7 @@ def live_session_may_be_ours( `main`-created run `ctl-<16 hex>` owns a genuine agent session distinct from the fixed name (measured: killing it exactly leaves `bmad-loop-ctl` alive), and the shape discount destroyed its run dir without ever querying - the mux. A namespace probe that cannot answer degrades to the fixed name - alone — the *smaller* discount, which blocks more, the safe direction. A - discount, not a proof source: it removes non-evidence, and never clears a + the mux. A discount, not a proof source: it removes non-evidence, and never clears a removal on transport testimony. **Transport-owned name comparison.** Every comparison goes through @@ -3276,8 +3189,8 @@ def live_session_may_be_ours( instead of printing it, and the fault is reported here in the same words as the raise. Left ``None``, the listing keeps its own stderr warning and this function adds nothing: still exactly one line per fault on each route. - The ``ctl_session_for`` and tag-read arms stay silent on - purpose: both degrade toward refusal, the safe direction.""" + The tag-read arm stays silent on purpose: it degrades toward refusal, + the safe direction.""" try: mux = get_multiplexer() except MultiplexerError as exc: @@ -3287,12 +3200,7 @@ def live_session_may_be_ours( return False key = mux.session_name_key name = session_name(run_id) - control = {CTL_SESSION} - try: - control.add(ctl_session_for(project, mux)) - except MultiplexerError: - pass # namespace unanswerable: only the fixed name is knowable - if key(name) in {key(c) for c in control}: + if key(name) == key(ctl_session_for(project, mux)): return False def folded(fault: str) -> None: diff --git a/src/bmad_loop/tui/app.py b/src/bmad_loop/tui/app.py index 4c0d09c2..97960857 100644 --- a/src/bmad_loop/tui/app.py +++ b/src/bmad_loop/tui/app.py @@ -2,10 +2,9 @@ Observer/launcher only: the TUI never runs engines in-process. Run control (r/s/e) launches detached bmad-loop processes in the control session via -tui.launch (bmad-loop-ctl on tmux; a per-registry name on psmux, which the -launch toasts print). Dry runs are captured into a text modal; validate -renders its `--json` document into a findings modal (falling back to the text -one), so the verdict is the document's `ok` rather than an exit code. +tui.launch (bmad-loop-ctl, which the launch toasts print). Dry runs are +captured into a text modal; validate renders its `--json` document into a +findings modal (falling back to the text one), so the verdict is the document's `ok` rather than an exit code. The g binding opens the policy.toml settings editor. The ctrl+p command palette lists every key-bound command live on the current screen or modal (tui.commands). """ diff --git a/src/bmad_loop/tui/launch.py b/src/bmad_loop/tui/launch.py index 696d4867..f50359f1 100644 --- a/src/bmad_loop/tui/launch.py +++ b/src/bmad_loop/tui/launch.py @@ -1,11 +1,11 @@ """Detached launching of bmad-loop commands for the TUI. The TUI never runs engines in-process: run/sweep/resume are launched in new -windows of a dedicated control session (bmad-loop-ctl on tmux, a per-registry -name on psmux — see the CTL_SESSION comment below) so they survive TUI exit, -and the dashboard observes them through run-dir artifacts exactly like runs -started from a plain shell. Fast read-only commands (validate, ---dry-run) are captured instead, for display in a modal. +windows of a dedicated control session (bmad-loop-ctl — see the CTL_SESSION +comment below) so they survive TUI exit, and the dashboard observes them +through run-dir artifacts exactly like runs started from a plain shell. Fast +read-only commands (validate, --dry-run) are captured instead, for display in +a modal. No textual imports here — everything drives the multiplexer seam (or a plain subprocess for the captured read-only commands) and is unit-testable. @@ -41,16 +41,11 @@ ) CTL_SESSION = runs.CTL_SESSION -# The control-session NAME is the transport's business, resolved per call -# through `runs.ctl_session_for(project)`: the fixed name on tmux, where one -# server serves the machine and the session really is machine-wide (scoped by -# the per-window PROJECT_OPTION tag below), and a per-registry name on psmux, -# whose duplicate-server mutex is keyed on the session name alone, across -# every registry in the login session (`Local\` is a per-login-session object -# namespace) — so a fixed name would let only ONE registry there hold a control -# session and every other project's launch would fail as a duplicate. -# The constant survives as the fixed base name (display fallbacks, tmux argv -# pins); anything that addresses a live session resolves the name instead. +# Anything that addresses a live control session resolves its name through +# `runs.ctl_session_for(project)` rather than this constant: on tmux the one +# session is machine-wide (scoped by the per-window PROJECT_OPTION tag below), +# on psmux each project's registry holds its own session of this name. The +# constant serves display fallbacks and tmux argv pins. # control-session windows are named - (see start_detached) _CTL_WINDOW_RE = re.compile(r"^(?:run|sweep|resume|resolve)-(.+)$") @@ -78,8 +73,7 @@ def agent_session_exists(session: str) -> bool: concerned. Saying why is the difference between that and "no session". Kept off :func:`session_exists` itself, which also answers for the control - session — a per-project name already (`runs.ctl_session_for`), whose - session carries no project tag at all.""" + session, which carries no project tag at all (its windows do).""" if not session_exists(session): return False refusal = runs.foreign_session_refusal(session, get_multiplexer()) @@ -669,8 +663,9 @@ def in_ctl_session() -> bool: current_session() is None whenever this process is not inside the selected multiplexer, so no direct TMUX/HERDR_* env sniffing happens here. The shape predicate rather than one project's resolved name: the question its - callers ask is "am I in A control session", and on a namespacing transport - the name carries a registry suffix (runs.ctl_session_for).""" + callers ask is "am I in A control session", and a control session started + by an earlier release on psmux carries a registry suffix + (runs.is_ctl_session_name).""" session = current_session() return session is not None and runs.is_ctl_session_name(session) diff --git a/tests/test_multiplexer.py b/tests/test_multiplexer.py index a2551523..5847f7d9 100644 --- a/tests/test_multiplexer.py +++ b/tests/test_multiplexer.py @@ -16,7 +16,6 @@ import pytest from conftest import needs_strict_codec -from bmad_loop import envvars from bmad_loop.adapters import multiplexer, tmux_base from bmad_loop.adapters.base import SessionSpec from bmad_loop.adapters.generic import GenericAdapter @@ -1766,8 +1765,8 @@ def __init__(self, **by_scope: tuple[int, str, str]): def __call__(self, argv, **_k): self.calls.append(list(argv)) - assert argv[2] == "show-environment" - rc, out, err = self.by_scope[argv[3].lstrip("-")] + verb = argv.index("show-environment") + rc, out, err = self.by_scope[argv[verb + 1].lstrip("-")] return subprocess.CompletedProcess(argv, rc, stdout=out, stderr=err) @@ -1856,23 +1855,105 @@ def run(argv, **_k): assert mux.inherited_env("ctl", "HOME") is None -def test_psmux_inherited_env_is_unknown_and_asks_nothing(monkeypatch): - """psmux keeps the seam default: its `show-environment` ignores the name and - hides inherited values, so parsing it as tmux's would read every inherited - variable as unset. Unknown, no spawn, nothing reported. +@pytest.mark.parametrize( + ("replies", "expected", "asked"), + [ + pytest.param({"t": (0, "BMAD_LOOP_STATE_DIR=/s1\n", "")}, "/s1", ["t"], id="session-set"), + pytest.param({"t": (0, "Bmad_Loop_State_Dir=/s1\n", "")}, "/s1", ["t"], id="stored-case"), + pytest.param({"t": (0, "BMAD_LOOP_STATE_DIR=\n", "")}, "", ["t"], id="set-empty"), + pytest.param({"t": (0, "-bmad_loop_state_dir\n", "")}, UNSET, ["t"], id="removed"), + pytest.param( + {"t": _MISS, "g": (0, "Bmad_Loop_State_Dir=/g\n", "")}, + "/g", + ["t", "g"], + id="global-hit", + ), + pytest.param({"t": _MISS, "g": _MISS}, UNSET, ["t", "g"], id="global-miss"), + pytest.param( + {"t": _MISS, "g": (0, "-BMAD_LOOP_STATE_DIR\n", "")}, + UNSET, + ["t", "g"], + id="global-removed", + ), + ], +) +def test_psmux_inherited_env_parses_show_environment(monkeypatch, replies, expected, asked): + """psmux answers tmux's replies since psmux/psmux#775, with two differences + measured on 3.4.0: the global scope is targeted at the session (one server + per session, so an untargeted `-g` answers from whichever server it lands + on), and the echoed name is the stored spelling, so it is matched + case-insensitively. Nothing reaches `on_fault`. + + Ablation: drop `fold_case=True` and the stored-case, removed and global-hit + rows fail; drop the `-g` target and the argv assertion fails.""" + from bmad_loop.adapters.psmux_backend import PsmuxMultiplexer + + fake = _EnvReplies(**replies) + monkeypatch.setattr(tmux_base.subprocess, "run", fake) + faults: list[str] = [] + + got = PsmuxMultiplexer().inherited_env("ctl", "BMAD_LOOP_STATE_DIR", on_fault=faults.append) + + assert got == expected and type(got) is type(expected) + assert faults == [] + scopes = {"t": ["-t", "=ctl"], "g": ["-g", "-t", "=ctl"]} + assert fake.calls == [ + ["psmux", "show-environment", *scopes[s], "BMAD_LOOP_STATE_DIR"] for s in asked + ] - Ablation: move `inherited_env` from `TmuxMultiplexer` to `BaseTmuxBackend` - and this fails on the spawn.""" + +@pytest.mark.parametrize( + "failure", + [ + pytest.param((1, "", "psmux: no server running on session 'ctl'\n"), id="no-server"), + pytest.param((1, "", "unknown variable: OTHER\n"), id="miss-for-another-name"), + pytest.param((0, "PATH=C:/x\n", ""), id="another-name"), + pytest.param((0, "", ""), id="empty-reply"), + pytest.param(subprocess.TimeoutExpired(["psmux"], 5), id="timeout"), + pytest.param(FileNotFoundError("psmux"), id="missing-binary"), + ], +) +def test_psmux_inherited_env_reports_a_failed_query_as_unknown(monkeypatch, failure): + """A query psmux could not answer is Unknown (`None`) AND one `on_fault` + call, never a silent Unknown and never raised. The case fold widens the + name match only: a reply for another name is still a fault. + + Ablation: drop the `on_fault` call from `_env_fault` and every row fails on + the fault count.""" + from bmad_loop.adapters.psmux_backend import PsmuxMultiplexer + + def run(argv, **_k): + if isinstance(failure, BaseException): + raise failure + rc, out, err = failure + return subprocess.CompletedProcess(argv, rc, stdout=out, stderr=err) + + monkeypatch.setattr(tmux_base.subprocess, "run", run) + faults: list[str] = [] + mux = PsmuxMultiplexer() + + assert mux.inherited_env("ctl", "HOME", on_fault=faults.append) is None + assert len(faults) == 1 and "show-environment" in faults[0] + assert mux.inherited_env("ctl", "HOME") is None + + +def test_psmux_inherited_env_reports_a_refused_registry_root(monkeypatch): + """The backend's own refusal to spawn under a relative `PSMUX_DATA_DIR` + raises `TmuxError` from `_run`; the query reports it rather than raising. + + Ablation: drop `TmuxError` from `_show_environment`'s except and this + raises.""" from bmad_loop.adapters.psmux_backend import PsmuxMultiplexer def no_spawn(argv, **_k): - raise AssertionError(f"psmux inherited_env spawned {argv}") + raise AssertionError(f"spawned {argv} under a refused registry root") monkeypatch.setattr(tmux_base.subprocess, "run", no_spawn) + monkeypatch.setenv("PSMUX_DATA_DIR", "relative") faults: list[str] = [] - got = PsmuxMultiplexer().inherited_env("ctl", envvars.STATE_DIR, on_fault=faults.append) - assert got is None - assert faults == [] + + assert PsmuxMultiplexer().inherited_env("ctl", "HOME", on_fault=faults.append) is None + assert len(faults) == 1 and "PSMUX_DATA_DIR" in faults[0] def test_inherited_env_seam_default_is_unknown_for_an_out_of_tree_backend(): diff --git a/tests/test_psmux_backend.py b/tests/test_psmux_backend.py index 16121e8e..cc4202e2 100644 --- a/tests/test_psmux_backend.py +++ b/tests/test_psmux_backend.py @@ -665,20 +665,16 @@ def test_available_requires_psmux_pwsh_and_supported_version(monkeypatch): "which", lambda name: f"C:\\bin\\{name}.exe" if name in ("psmux", "pwsh") else None, ) - monkeypatch.setattr(PsmuxMultiplexer, "version", lambda self: "tmux 3.3.8") - assert PsmuxMultiplexer().available() is True - - # 3.3.7 and older are refused: 3.3.6 force-kills recycled PIDs on teardown, - # and 3.3.7 lacks the fixes this backend's verbs now assume. - monkeypatch.setattr(PsmuxMultiplexer, "version", lambda self: "tmux 3.3.7") - assert PsmuxMultiplexer().available() is False - - monkeypatch.setattr(PsmuxMultiplexer, "version", lambda self: "tmux 3.3.6") - assert PsmuxMultiplexer().available() is False - monkeypatch.setattr(PsmuxMultiplexer, "version", lambda self: "tmux 3.4.0") assert PsmuxMultiplexer().available() is True + # 3.3.8 and older are refused: 3.3.6 force-kills recycled PIDs on teardown, + # 3.3.7 lacks the fixes this backend's verbs assume, and 3.3.8 unsets a key + # on a `-`-leading value (psmux/psmux#583). + for refused in ("3.3.8", "3.3.7", "3.3.6"): + monkeypatch.setattr(PsmuxMultiplexer, "version", lambda self, v=refused: f"tmux {v}") + assert PsmuxMultiplexer().available() is False + # multi-digit segments compare numerically, not lexicographically monkeypatch.setattr(PsmuxMultiplexer, "version", lambda self: "tmux 3.10.0") assert PsmuxMultiplexer().available() is True @@ -687,10 +683,10 @@ def test_available_requires_psmux_pwsh_and_supported_version(monkeypatch): # a suffixed newer release still clears the strictly-greater gate; a # suffixed refused one still fails it (the suffix is not read at all) - monkeypatch.setattr(PsmuxMultiplexer, "version", lambda self: "tmux 3.3.8-rc0") + monkeypatch.setattr(PsmuxMultiplexer, "version", lambda self: "tmux 3.3.9-rc0") assert PsmuxMultiplexer().available() is True - monkeypatch.setattr(PsmuxMultiplexer, "version", lambda self: "tmux 3.3.7-rc0") + monkeypatch.setattr(PsmuxMultiplexer, "version", lambda self: "tmux 3.3.8-rc0") assert PsmuxMultiplexer().available() is False # a two-part compat version (tmux's own format) reads as patch 0 @@ -715,7 +711,7 @@ def test_available_composes_real_version_probe(monkeypatch): lambda name: f"C:\\bin\\{name}.exe" if name in ("psmux", "pwsh") else None, ) monkeypatch.setattr(tmux_base.shutil, "which", lambda name: f"C:\\bin\\{name}.exe") - rec = _RecordRun(stdout="tmux 3.3.8\n") + rec = _RecordRun(stdout="tmux 3.4.0\n") monkeypatch.setattr(tmux_base.subprocess, "run", rec) assert PsmuxMultiplexer().available() is True assert rec.argv == ["psmux", "-V"] @@ -736,7 +732,7 @@ def test_available_parses_the_gate_out_of_a_real_two_line_probe(monkeypatch): lambda name: f"C:\\bin\\{name}.exe" if name in ("psmux", "pwsh", "tmux") else None, ) - for release, expected in (("3.3.8", True), ("3.3.7", False)): + for release, expected in (("3.4.0", True), ("3.3.8", False)): rec = _RecordRun(stdout=f"tmux {release}\npsmux {release} (66cf613 2026-08-18)\n") monkeypatch.setattr(tmux_base.subprocess, "run", rec) mux = PsmuxMultiplexer() @@ -756,7 +752,7 @@ def test_available_caches_version_gate_per_instance(monkeypatch): def probe(self): nonlocal calls calls += 1 - return "tmux 3.3.8" + return "tmux 3.4.0" monkeypatch.setattr(PsmuxMultiplexer, "version", probe) mux = PsmuxMultiplexer() @@ -803,7 +799,7 @@ def fake(argv, **kwargs): if argv[0] == PINNED_PWSH: out = reported + "\n" else: - out = "tmux 3.3.8\npsmux 3.3.8\n" if argv[1:] == ["-V"] else "@2\n" + out = "tmux 3.4.0\npsmux 3.4.0\n" if argv[1:] == ["-V"] else "@2\n" return subprocess.CompletedProcess(argv, 0, stdout=out, stderr="") monkeypatch.setattr(tmux_base.subprocess, "run", fake) @@ -1693,7 +1689,6 @@ def test_kill_window_unroutable_target_skips_cleanup_but_kills(monkeypatch): "bad\nline", # the listing is parsed line-by-line — a break splits the value "bad\rline", # splitlines() cuts on `\r` too, not just `\n` "", # empty write is a silent server-side no-op; unset exists for this - "-flag", # dropped as a flag server-side, or flips to unset (psmux/psmux#583) " C:\\p x", # leading space survives the wire but this backend's reads strip "C:\\p x ", # trailing space, same round-trip loss ], @@ -1724,6 +1719,8 @@ def test_set_window_option_refuses_untransportable_values(monkeypatch, capsys, v "\\\\srv\\share My Proj", # spaced UNC — `\\` no longer collapses (#547) "C:\\dir with space\\", # trailing `\` no longer eats the closing quote "x\u00a0y", # non-ASCII whitespace no longer splits server-side (#536) + "-u", # a `-`-leading value is stored, not read as a flag (psmux/psmux#583) + "-flag", ], ) def test_set_window_option_accepts_wire_safe_values(monkeypatch, value): @@ -1750,7 +1747,6 @@ def test_set_window_option_write_failure_warns(monkeypatch, capsys): 'C:\\a"b\\proj', # `"` cannot survive the listing's one-quote-pair strip "bad\nline", # the listing is parsed line-by-line "", # an empty write is a silent server-side no-op - "-flag", # dropped as a flag server-side (psmux/psmux#583) ], ) def test_set_session_option_refuses_untransportable_values(monkeypatch, capsys, value): @@ -1778,11 +1774,12 @@ def test_set_session_option_refuses_untransportable_values(monkeypatch, capsys, "C:\\projects\\my proj\\", # trailing `\` no longer eats the closing quote "\\\\srv\\share name\\proj", # spaced UNC — `\\` no longer collapses (#547) "C:\\Users\\O'Brien\\proj", # unspaced `'` no longer opens a quote + "-u", # a `-`-leading value is stored, not read as a flag (psmux/psmux#583) ], ) def test_set_session_option_accepts_transportable_values(monkeypatch, value): - # All eight round-trip unchanged on psmux 3.3.8; a gate that refused them - # would untag ordinary Windows project paths. + # All round-trip unchanged on the supported psmux; a gate that refused + # them would untag ordinary Windows project paths. rec_ = _option_fake(monkeypatch) PsmuxMultiplexer().set_session_option("bmad-loop-x", "@bmad_project", value) assert rec_.argv[1:] == ["set-option", "-t", "bmad-loop-x", "@bmad_project", value] @@ -2012,6 +2009,7 @@ def fake(argv, **kwargs): "C:\\dir with space\\", "\\\\srv\\share My Proj", "x\u00a0y", + "-u", ], ) def test_accepted_values_round_trip_through_the_listing_parse(monkeypatch, value): @@ -2062,10 +2060,8 @@ def fake(argv, **kwargs): monkeypatch.setattr(runs, "engine_alive", lambda _dir: False) candidates, undetermined = launch._ctl_window_candidates(tmp_path) - # The scan resolves the per-registry ctl name (runs.ctl_session_for) on a - # namespacing backend, and the qualified candidate ids carry it. + # The qualified candidate ids carry the control session's name. ctl = runs.ctl_session_for(tmp_path, mux) - assert ctl.startswith("bmad-loop-ctl-") assert candidates == [(f"{ctl}:@2", "run-20260726-1")] assert captured == [f"{ctl}:@2"] assert undetermined == [] diff --git a/tests/test_psmux_live.py b/tests/test_psmux_live.py index 08a8886b..3e81d377 100644 --- a/tests/test_psmux_live.py +++ b/tests/test_psmux_live.py @@ -18,7 +18,7 @@ never read as a premise flip. The `test_adopted_*` probes are the ordinary-direction complement: each -exercises a behavior the 3.3.8 floor let the backend assume (several through +exercises a behavior a version floor let the backend assume (several through the backend verb itself, deliberately), so a red probe there means psmux regressed, not that a workaround became droppable. @@ -45,7 +45,7 @@ from bmad_loop import runs from bmad_loop.adapters import psmux_backend, tmux_base -from bmad_loop.adapters.multiplexer import parked_screen +from bmad_loop.adapters.multiplexer import UNSET, parked_screen from bmad_loop.adapters.psmux_backend import PsmuxMultiplexer from bmad_loop.tui import launch @@ -112,9 +112,8 @@ def test_prune_kills_only_the_owning_projects_window(tmp_path: Path, monkeypatch proc = mux._run(["set-option", "-t", session, foreign, "user"], check=False) assert proc.returncode == 0, proc.stderr - # The ctl-session NAME is resolved per (project, transport) since the - # per-registry rename (runs.ctl_session_for); pin the resolver to this - # test's own session so the prune sweeps the windows minted above. + # Pin the ctl-session resolver (runs.ctl_session_for) to this test's own + # session so the prune sweeps the windows minted above. monkeypatch.setattr(runs, "ctl_session_for", lambda _p, _m=None: session) monkeypatch.setattr(launch, "get_multiplexer", lambda: mux) # Pin "outside any pane": when pytest itself runs inside psmux the @@ -633,51 +632,15 @@ def test_premise_target_less_display_message_ignores_tmux_pane(probe): ) -def test_premise_a_dash_leading_option_value_unsets_the_key(probe): - """`set-option -t S @k -u` reads the value as the unset flag: rc 0, and the - key is gone. That silent data loss is why `_transportable` refuses every - `-`-leading value (psmux/psmux#583). +def test_adopted_two_registries_each_hold_a_session_of_one_name(tmp_path): + """psmux's one-server-per-name guard follows `PSMUX_DATA_DIR` + (psmux/psmux#599), so while root A holds a session, root B creates its own + of the same name and both answer in their own registry. That is what lets + every project's registry hold the fixed `bmad-loop-ctl` + (`runs.ctl_session_for`, #733). - Red means the installed psmux stores the value verbatim, and that refusal is - droppable — but only behind the `_LAST_UNSUPPORTED` gate, set to the last - release WITHOUT the fix (the gate is exclusive: ``available()`` admits only - versions above it), since an older admitted build still unsets. - """ - mux, session, _ = probe - seeded = mux._run(["set-option", "-t", session, "@k", "v"], check=False) - assert seeded.returncode == 0, f"probe setup: seeding @k failed: {seeded.stderr.strip()!r}" - seen = mux._run(["show-options", "-qv", "-t", session, "@k"], check=False) - assert seen.stdout.strip() == "v", f"probe setup: @k did not read back: {seen.stdout!r}" - written = mux._run(["set-option", "-t", session, "@k", "-u"], check=False) - assert written.returncode == 0, ( - "psmux now refuses a `-`-leading option value outright instead of silently " - f"unsetting the key (psmux/psmux#583): {written.stderr.strip()!r}" - ) - got = mux._run(["show-options", "-qv", "-t", session, "@k"], check=False) - assert got.returncode == 0, f"probe setup: reading @k back failed: {got.stderr.strip()!r}" - assert got.stdout.strip() == "", ( - "psmux now stores a `-`-leading option value verbatim (psmux/psmux#583, read " - f"back {got.stdout.strip()!r}) — the `-` refusal in psmux_backend._transportable " - "is droppable once _LAST_UNSUPPORTED is the last release without the fix" - ) - - -def test_premise_one_session_name_is_held_across_registries(tmp_path): - """psmux's one-server-per-name guard is keyed on the session name alone, not - the registry root, so while root A holds a session a second root cannot - create one of the same name. That collision is why `runs.ctl_session_for` - suffixes the control-session name per registry (psmux/psmux#599). - - Red means the guard follows `PSMUX_DATA_DIR` and the per-registry suffix is - no longer forced by psmux — droppable only behind the `_LAST_UNSUPPORTED` - gate, set to the last release WITHOUT the fix (the gate is exclusive). - - Only a refusal with nothing registered in B counts as the collision. A - failed create whose server registers anyway is the client's readiness - deadline under load, not the guard, and a fixed build can produce it too. - Ceiling: B is read once, so a server registering after that read passes - as the collision. That errs green, never red: on a fixed release it delays - the flip to the next unloaded run instead of inventing one. + Red means the guard is keyed on the name alone again: every project's TUI + launch but the first fails as a duplicate cold-spawn. Root A is torn down before root B, and that order is load-bearing: B's never-seen teardown ends in a process-table kill by session token, which @@ -710,37 +673,29 @@ def test_premise_one_session_name_is_held_across_registries(tmp_path): ["new-session", "-d", "-s", session, "-c", str(tmp_path)], check=False, env=env_b ) b_seen = _plain_has_session(mux, session, env=env_b) - # A dead A would let B's create succeed on any build: not a flip. - assert _plain_has_session( - mux, session, env=env_a - ), "probe setup: root A's session died while root B created its own" + a_kept = _plain_has_session(mux, session, env=env_a) finally: _teardown_probe_session(mux, session, env_a, known_created=a_seen) finally: _teardown_probe_session(mux, session, env_b, known_created=b_seen) - assert created_b.returncode == 0 or not b_seen, ( - "probe setup: root B's create reported failure but its session registered " - f"anyway — a readiness timeout, not the name guard: {created_b.stderr.strip()!r}" - ) - assert created_b.returncode != 0, ( - "psmux now lets a second registry hold a session name another registry's live " - "server owns (psmux/psmux#599) — the per-registry control-session suffix in " - "runs.ctl_session_for (#733) retires, but only through a _LAST_UNSUPPORTED bump " - "to the last release without the fix, never a deletion: an older admitted build " - "still collides" + assert created_b.returncode == 0 and b_seen, ( + "psmux refused a second registry a session name another registry's live " + f"server owns (psmux/psmux#599, rc {created_b.returncode}: " + f"{created_b.stderr.strip()!r}) — the fixed control-session name in " + "runs.ctl_session_for (#733) admits one project per login session again" ) + assert a_kept, "root B's create took root A's session down" -def test_premise_has_session_reaps_a_live_registry_entry_on_a_connect_timeout( - probe, psmux_data_root -): - """A `has-session` whose 500 ms connect times out deletes the session's - `.port` even though its server is alive — the reap window the accepted - ceiling in `runs.live_session_may_be_ours` documents (psmux/psmux#622). +def test_adopted_a_live_registry_entry_survives_a_slow_connect(probe, psmux_data_root): + """A `has-session` whose 500 ms connect times out leaves a live server's + `.port` in place: it reaps only on a refusal or a dead pid anchor + (psmux/psmux#622). That is why the ceiling in + `runs.live_session_may_be_ours` is the PATH case alone (#754). - Red means a timeout no longer reaps, and that ceiling's psmux half retires — - only behind the `_LAST_UNSUPPORTED` gate, set to the last release WITHOUT - the fix (the gate is exclusive), since an older admitted build still reaps. + Red means a slow connect reaps a live entry again, and a successful listing + can omit a live session with no fault reported: the removal guard's + ceiling grows that case back. Determinism: the forged port is one this process has bound but never listens on, which on Windows times out rather than refuses (checked first: @@ -798,11 +753,10 @@ def test_premise_has_session_reaps_a_live_registry_entry_on_a_connect_timeout( assert ( asked.returncode != 0 ), "probe setup: has-session answered through a port nothing listens on" - assert reaped, ( - "psmux has-session no longer reaps a live session's .port on a connect " - f"timeout (psmux/psmux#622, .port kept as {left!r}) — the reap-window ceiling " - "prose in runs.live_session_may_be_ours (#754) narrows once _LAST_UNSUPPORTED " - "is the last release without the fix" + assert not reaped, ( + "psmux has-session reaped a live session's .port on a connect timeout " + "(psmux/psmux#622) — runs.live_session_may_be_ours can read a live session " + "as absent again, beyond the PATH case its ceiling names (#754)" ) finally: dead.close() @@ -811,136 +765,69 @@ def test_premise_has_session_reaps_a_live_registry_entry_on_a_connect_timeout( port_file.write_text(real, encoding="utf-8") -def test_premise_pane_path_is_rebuilt_from_the_registry(tmp_path): - """A pane does not inherit the PATH of the process that started its server: - psmux rebuilds it from the registry (psmux/psmux#773). That is why the - parked-window trailer pins the absolute psmux path instead of trusting a - bare `psmux` inside the pane (#877). - - Red means the pane kept the launcher's PATH entry, and the pin can retire, - but only behind the `_LAST_UNSUPPORTED` gate, set to the last release - WITHOUT the fix (the gate is exclusive), since an older admitted build - still rebuilds PATH. +def test_adopted_show_environment_answers_one_name_per_session_scope(probe): + """`show-environment` answers for the one NAME asked (psmux/psmux#775, + fixed in 3.4.0), which is what `PsmuxMultiplexer.inherited_env` parses: + `NAME=value` from the session scope, `-NAME` for a removal, `unknown + variable: NAME` at rc 1 for a miss, and a `-g -t =` global scope + that is that session's server start environment. The reply echoes the name + as stored, so a differently-cased query must still match it. - Its own fresh registry root, so the session is a cold start from this - test's environment and never a claimed warm server whose first pane was - spawned before the claim. A launcher-only variable rides along as the - control: a pane missing THAT never saw this environment at all. - """ - mux = PsmuxMultiplexer() - if not mux.available(): - pytest.skip("psmux present but not an admitted version") - root = tmp_path / "registry" - root.mkdir() # pre-created: see psmux_data_root for the measured cost - marker = tmp_path / "path-marker" - marker.mkdir() - report = tmp_path / "pane-env.txt" - staging = tmp_path / "pane-env.tmp" - mark = uuid.uuid4().hex - env = _new_session_env() - env["PSMUX_DATA_DIR"] = str(root) - env["PATH"] = f"{marker};{env.get('PATH', '')}" - env["BMAD_LOOP_PROBE_MARK"] = mark - # Staged then moved, so a poll never reads a half-written report. The - # source travels as -EncodedCommand: psmux re-parses a command line, and - # base64 is the one shape that survives it whatever the paths hold. - source = ( - f"Set-Content -LiteralPath {psmux_backend._pwsh_quote(str(staging))} " - '-Value ("$env:BMAD_LOOP_PROBE_MARK|" + $env:PATH); ' - f"Move-Item -LiteralPath {psmux_backend._pwsh_quote(str(staging))} " - f"-Destination {psmux_backend._pwsh_quote(str(report))}; Start-Sleep 60" - ) - encoded = base64.b64encode(source.encode("utf-16-le")).decode("ascii") - session = f"bmad-loop-path-probe-{uuid.uuid4().hex[:8]}" - seen = False - text = "" - try: - created = mux._run( - [ - "new-session", - "-d", - "-s", - session, - "-c", - str(tmp_path), - "pwsh", - "-NoProfile", - "-EncodedCommand", - encoded, - ], - check=False, - env=env, - ) - assert ( - created.returncode == 0 - ), f"probe setup: probe session creation failed: {created.stderr.strip()!r}" - seen = _plain_has_session(mux, session, env=env) - deadline = time.monotonic() + 15 - while time.monotonic() < deadline and not report.exists(): - time.sleep(0.25) - assert report.exists(), "probe setup: the probe pane never reported its environment" - text = report.read_text(encoding="utf-8-sig").strip() - finally: - _teardown_probe_session(mux, session, env, known_created=seen) - got_mark, _, pane_path = text.partition("|") - assert got_mark == mark, ( - "probe setup: the pane did not inherit the launcher's environment at all " - f"(control variable read {got_mark!r}) — it says nothing about PATH" - ) - entries = [entry.rstrip("\\").lower() for entry in pane_path.split(";")] - assert str(marker).lower() not in entries, ( - "psmux now gives a pane the launcher's PATH instead of rebuilding it from the " - "registry (psmux/psmux#773) — the pinned psmux path in the parked-window " - "trailer (#877, item 1 of #879) retires, but only through a _LAST_UNSUPPORTED " - "bump to the last release without the fix, never a deletion: an older admitted " - "build still rebuilds PATH" - ) - - -def test_premise_show_environment_ignores_the_name(probe): - """`show-environment -t NAME` answers with the session server's - whole listing at rc 0, whatever NAME is (psmux/psmux#775). A tmux-style - named query cannot be asked, which is why the psmux backend keeps the - seam's `inherited_env -> None` default (#660). - - Red means psmux answers for that one name, and a psmux `inherited_env` - can be adopted behind a `_LAST_UNSUPPORTED` bump to the last release - without the fix. Two adoption notes measured on the fixed build: an - untargeted `-g` answers from whichever session's server it lands on (one - server per session), and `-g PATH` answers `Path=...`, the spelling the - environment block stores. + Raw argv for the reply shape, then the backend verb itself. Red means + psmux went back to answering with a listing, and the backend's query + turns every answer into a fault (or a wrong value): the stale state-root + note goes silent on Windows. """ mux, session, _ = probe name = "BMAD_LOOP_PROBE_SES" - stored = mux._run(["set-environment", "-t", session, name, "session-val"], check=False) - assert stored.returncode == 0, f"probe setup: set-environment failed: {stored.stderr.strip()!r}" - asked = mux._run(["show-environment", "-t", session, name], check=False) - assert ( - asked.returncode == 0 - ), f"probe setup: the named show-environment failed: {asked.stderr.strip()!r}" - lines = [line.strip() for line in asked.stdout.splitlines() if line.strip()] - # Either shape carries the line; its absence means the write never landed - # or the reply came from another server, and neither is the premise. + gone = "BMAD_LOOP_PROBE_RM" + for argv in ( + ["set-environment", "-t", session, name, "session-val"], + ["set-environment", "-t", session, "-r", gone], + ): + stored = mux._run(argv, check=False) + assert stored.returncode == 0, f"probe setup: {argv!r} failed: {stored.stderr.strip()!r}" + inherited = os.environ.get("LOCALAPPDATA") + assert inherited, "probe setup: this process has no LOCALAPPDATA for the server to inherit" + + asked = mux._run(["show-environment", "-t", f"={session}", name.lower()], check=False) + assert (asked.returncode, asked.stdout.splitlines()) == (0, [f"{name}=session-val"]), ( + "psmux no longer answers a named session-scope show-environment for that one " + f"name (psmux/psmux#775): rc {asked.returncode}, {asked.stdout[:200]!r}" + ) + glob = mux._run(["show-environment", "-g", "-t", f"={session}", "localappdata"], check=False) + lines = glob.stdout.splitlines() assert ( - f"{name}=session-val" in lines - ), f"probe setup: the reply does not carry the stored variable: {lines!r}" - assert lines != [f"{name}=session-val"], ( - "psmux now answers show-environment NAME for that one name (psmux/psmux#775) — " - "a psmux inherited_env can be adopted (#660) behind a _LAST_UNSUPPORTED bump to " - "the last release without the fix: send `show-environment -g -t = NAME` " - "(an untargeted -g answers from whichever session's server it lands on) and match " - "the returned name case-insensitively (`-g PATH` answers `Path=...`)" + glob.returncode == 0 + and len(lines) == 1 + and lines[0].lower() == f"localappdata={inherited}".lower() + ), ( + "psmux no longer answers a named, session-targeted `-g` query from that " + f"session's server start environment: rc {glob.returncode}, {glob.stdout[:200]!r}" ) + faults: list[str] = [] + answers = { + query: mux.inherited_env(session, query, on_fault=faults.append) + for query in (name.lower(), gone, "BMAD_LOOP_PROBE_NOPE", "localappdata") + } + assert answers == { + name.lower(): "session-val", + gone: UNSET, + "BMAD_LOOP_PROBE_NOPE": UNSET, + "localappdata": inherited, + }, f"inherited_env misread psmux's show-environment replies: {answers!r}" + assert faults == [], f"inherited_env reported faults on a healthy session: {faults!r}" + # ------------------------------------------------- adopted-behavior probes # # The mirror image of the premise probes above. Those guarded workarounds and -# went red when upstream made one droppable; these guard the four behaviors the -# 3.3.8 floor let this backend START assuming, and go red if a later psmux takes -# one back. Without them the seam's only evidence for those four is a mocked -# argv shape, which cannot notice a regression in the binary at all. Ordinary -# semantics: red means broken. +# went red when upstream made one droppable; these guard the behaviors a version +# floor let this backend START assuming, and go red if a later psmux takes one +# back. Without them the seam's only evidence for those is a mocked argv shape, +# which cannot notice a regression in the binary at all. Ordinary semantics: red +# means broken. def test_adopted_pipe_pane_delivers_pane_bytes_through_the_flag_transport(probe, tmp_path): @@ -1048,12 +935,158 @@ def await_images(at_least: int) -> int: os.rmdir(link) # the junction only, never the install it points at +def test_adopted_a_dash_leading_option_value_is_stored(probe): + """A `-`-leading option value is stored verbatim, not read as the unset + flag (psmux/psmux#583), on both option channels and through the backend + verbs themselves, since `_transportable` no longer refuses one. Red means + psmux drops or unsets such a value again, and the refusal is needed back. + + Each key is seeded first, so an unset (the 3.3.8 behavior: rc 0 and the + key gone) reads as a lost value, never as a write that did not happen. + """ + mux, session, windows = probe + for value in ("-u", "-flag"): + seeded = mux._run(["set-option", "-t", session, "@k", "seed"], check=False) + assert seeded.returncode == 0, f"probe setup: seeding @k failed: {seeded.stderr.strip()!r}" + mux.set_session_option(session, "@k", value) + got = mux._run(["show-options", "-qv", "-t", session, "@k"], check=False) + assert got.returncode == 0, f"probe setup: reading @k back failed: {got.stderr.strip()!r}" + assert got.stdout.strip() == value, ( + f"psmux no longer stores the `-`-leading option value {value!r} (psmux/psmux#583, " + f"read back {got.stdout.strip()!r}) — psmux_backend._transportable must refuse " + "`-`-leading values again" + ) + window = windows[0] + key = mux._scoped_option_key("@r", window.rsplit("@", 1)[1]) + mux.set_window_option(window, "@r", "-u") + options = mux._scoped_options(session) + assert options is not None, "probe setup: the scoped option listing failed" + assert options.get(key) == "-u", ( + "psmux no longer stores a `-`-leading per-window option value (psmux/psmux#583, " + f"read back {options.get(key)!r}) — psmux_backend._transportable must refuse " + "`-`-leading values again" + ) + + +def test_adopted_a_pane_inherits_the_servers_start_path(tmp_path): + """A pane inherits the PATH of the process that started its server, and a + bare `psmux` in the pane resolves through it (psmux/psmux#773); 3.3.8 + rebuilt the pane's PATH from the registry instead. The parked-window + trailer keeps pinning psmux's absolute path (#877), because a pane's PATH + is the server starter's, not this launcher's; this row records the + inheritance that pin's retirement would rest on, and that #879 measured. + + The launcher's PATH reaches psmux only through a junction onto the + directory `psmux` resolves from, with that directory itself removed, so + the pane can find psmux through the launcher's entry alone. The pane runs + the installed pwsh by absolute path for the same reason. Red means psmux + rebuilds a pane's PATH again. + + Its own fresh registry root, so the session is a cold start from this + test's environment and never a claimed warm server whose first pane was + spawned before the claim. A launcher-only variable rides along as the + control: a pane missing THAT never saw this environment at all. + """ + mux = PsmuxMultiplexer() + if not mux.available(): + pytest.skip("psmux present but not an admitted version") + found = shutil.which("psmux") + assert found is not None, "probe setup: psmux does not resolve" + install_dir = os.path.dirname(os.path.abspath(found)) + root = tmp_path / "registry" + root.mkdir() # pre-created: see psmux_data_root for the measured cost + link = tmp_path / "launcher-only psmux" + made = subprocess.run( + ["cmd", "/c", "mklink", "/J", str(link), install_dir], + capture_output=True, + text=True, + timeout=tmux_base.TMUX_TIMEOUT_S, + ) + assert made.returncode == 0, f"probe setup: junction failed: {made.stderr.strip()!r}" + report = tmp_path / "pane-env.txt" + staging = tmp_path / "pane-env.tmp" + mark = uuid.uuid4().hex + env = _new_session_env() + env["PSMUX_DATA_DIR"] = str(root) + kept = [ + entry + for entry in env.get("PATH", "").split(";") + if entry and os.path.normcase(entry.rstrip("\\")) != os.path.normcase(install_dir) + ] + env["PATH"] = ";".join([str(link), *kept]) + env["BMAD_LOOP_PROBE_MARK"] = mark + # Staged then moved, so a poll never reads a half-written report. The + # source travels as -EncodedCommand: psmux re-parses a command line, and + # base64 is the one shape that survives it whatever the paths hold. + source = ( + f"Set-Content -LiteralPath {psmux_backend._pwsh_quote(str(staging))} " + '-Value ("$env:BMAD_LOOP_PROBE_MARK|" + ' + "(Get-Command psmux -CommandType Application -ErrorAction SilentlyContinue " + '| Select-Object -First 1).Source + "|" + $env:PATH); ' + f"Move-Item -LiteralPath {psmux_backend._pwsh_quote(str(staging))} " + f"-Destination {psmux_backend._pwsh_quote(str(report))}; Start-Sleep 60" + ) + encoded = base64.b64encode(source.encode("utf-16-le")).decode("ascii") + session = f"bmad-loop-path-probe-{uuid.uuid4().hex[:8]}" + seen = False + text = "" + try: + try: + created = mux._run( + [ + "new-session", + "-d", + "-s", + session, + "-c", + str(tmp_path), + _REAL_PWSH_PATH(mux), + "-NoProfile", + "-EncodedCommand", + encoded, + ], + check=False, + env=env, + ) + assert ( + created.returncode == 0 + ), f"probe setup: probe session creation failed: {created.stderr.strip()!r}" + seen = _plain_has_session(mux, session, env=env) + deadline = time.monotonic() + 15 + while time.monotonic() < deadline and not report.exists(): + time.sleep(0.25) + assert report.exists(), "probe setup: the probe pane never reported its environment" + text = report.read_text(encoding="utf-8-sig").strip() + finally: + _teardown_probe_session(mux, session, env, known_created=seen) + finally: + os.rmdir(link) # the junction only, never the install it points at + got_mark, _, rest = text.partition("|") + resolved, _, pane_path = rest.partition("|") + assert got_mark == mark, ( + "probe setup: the pane did not inherit the launcher's environment at all " + f"(control variable read {got_mark!r}) — it says nothing about PATH" + ) + entries = [os.path.normcase(entry.rstrip("\\")) for entry in pane_path.split(";")] + assert os.path.normcase(str(link)) in entries, ( + "psmux no longer gives a pane the PATH of the process that started its server " + "(psmux/psmux#773) — a bare `psmux` in a pane can find another build or none, " + "so the trailer's pinned psmux path (#877) cannot retire" + ) + assert resolved and os.path.normcase(os.path.dirname(resolved)) == os.path.normcase( + str(link) + ), ( + f"a bare `psmux` in the pane resolved {resolved!r}, not the build the launcher's " + "PATH holds — the trailer's pinned psmux path (#877) cannot retire" + ) + + def test_a_parked_trailer_runs_the_pinned_psmux_off_the_pane_path(probe, tmp_path, monkeypatch): """The parked-window trailer calls the absolute psmux the backend resolved, - not whatever the pane's PATH finds (#877): psmux builds a pane's PATH from - the registry (psmux/psmux#773), so a psmux reachable only through the - launching shell's PATH is missing in the pane. Here the pane's PATH is cut - to System32, and the pinned path is a junction holding a space, an + not whatever the pane's PATH finds (#877): a pane's PATH is that of the + process that started its server, or on a pre-floor psmux one rebuilt from + the registry, so a psmux reachable only through this launcher's PATH can be + missing in the pane. Here the pane's PATH is cut to System32, and the pinned path is a junction holding a space, an apostrophe and a doubled space onto the directory of the running server's own image. The trailer's last verb frees the return-target key, and every trailer error goes to $null, so the key going away is the proof that the diff --git a/tests/test_runs.py b/tests/test_runs.py index 2c5ff6dc..7a88bb96 100644 --- a/tests/test_runs.py +++ b/tests/test_runs.py @@ -2014,6 +2014,9 @@ def test_prunable_sessions_partitions(tmp_path, monkeypatch): "bmad-loop-untag-fin", # pre-upgrade session, no tag "bmad-loop-untag-orphan", # pre-upgrade, no tag, no run dir here "bmad-loop-ctl", # control session: never a candidate + # a control session an earlier release minted per registry, still + # running after the upgrade: tagged ours, and still never a candidate + "bmad-loop-ctl-0123456789abcdef", "unrelated", # not ours ] monkeypatch.setattr(runs, "mux_sessions", lambda: sessions) @@ -2025,6 +2028,7 @@ def test_prunable_sessions_partitions(tmp_path, monkeypatch): "bmad-loop-fin-1": mine, "bmad-loop-orphan-1": mine, "bmad-loop-other-1": "/some/other/project", + "bmad-loop-ctl-0123456789abcdef": mine, # untag-* and unrelated intentionally absent (no tag) }, ) @@ -7740,50 +7744,6 @@ def test_honouring_still_overrides_a_value_psmux_would_panic_on(tmp_path, monkey assert os.environ[runs.PSMUX_DATA_DIR] == derived -def test_ctl_session_for_follows_an_honoured_registry(tmp_path, monkeypatch): - """psmux's duplicate-server mutex is keyed on the session name alone, so the - control session in an honoured registry must not reuse the name the derived - registry's one holds: turning the opt-in on while the old control session - lives would otherwise make every TUI launch fail. With the derived root - settled, the name is unchanged. - - Ablate the honoured-root arm in `ctl_session_for` and the first assert - fails.""" - derived = str(runs.mux_registry_root(tmp_path)) - monkeypatch.setenv(runs.PSMUX_DATA_DIR, derived) - as_derived = runs.ctl_session_for(tmp_path, _NamespaceStub(True)) - monkeypatch.setenv(runs.PSMUX_DATA_DIR, str(tmp_path / "pinned")) - as_honoured = runs.ctl_session_for(tmp_path, _NamespaceStub(True)) - monkeypatch.setenv(runs.PSMUX_DATA_DIR, os.path.join(derived, "..", runs.MUX_REGISTRY_DIR)) - respelled = runs.ctl_session_for(tmp_path, _NamespaceStub(True)) - monkeypatch.delenv(runs.PSMUX_DATA_DIR) - unset = runs.ctl_session_for(tmp_path, _NamespaceStub(True)) - - assert as_honoured != as_derived - assert as_derived == unset == respelled - assert runs.is_ctl_session_name(as_honoured) - - -def test_ctl_session_for_reads_the_backends_root(tmp_path, monkeypatch): - """The honoured root is the one the backend's verbs resolve through, not - whatever the environment says: a bound instance answers for its own - registry, so its control session is named for that registry. - - Ablate (read `PSMUX_DATA_DIR` from the environment again) and the bound - instance is named for the derived registry instead.""" - derived = str(runs.mux_registry_root(tmp_path)) - pinned = str(tmp_path / "pinned") - monkeypatch.setenv(runs.PSMUX_DATA_DIR, pinned) - as_honoured = runs.ctl_session_for(tmp_path, _NamespaceStub(True)) - monkeypatch.setenv(runs.PSMUX_DATA_DIR, derived) - as_derived = runs.ctl_session_for(tmp_path, _NamespaceStub(True)) - - bound = runs.ctl_session_for(tmp_path, _NamespaceStub(True, root=pinned)) - - assert as_honoured != as_derived # the premise: the two roots name differently - assert bound == as_honoured - - def test_honouring_hands_the_derived_root_to_the_sweep(tmp_path, monkeypatch): """Turning the flag on moves the registry from the derived root to the pin, and sessions started before the switch are still in the derived one. It is @@ -7833,95 +7793,46 @@ def boom(): class _NamespaceStub: - """Duck-typed mux answering only what ctl_session_for consults: the - namespace question, and the registry root — the environment's, as the - primary psmux instance answers, unless bound to its own.""" + """Duck-typed mux answering only the namespace question, so an ablation + that restores a per-registry name has a namespacing transport to act on.""" - def __init__(self, namespaced, root=None): + def __init__(self, namespaced): self._namespaced = namespaced - self._root = root def has_registry_namespace(self): return self._namespaced def registry_root(self): - return self._root or os.environ.get(runs.PSMUX_DATA_DIR) + return os.environ.get(runs.PSMUX_DATA_DIR) -def test_ctl_session_for_is_fixed_without_a_registry_namespace(tmp_path): - """tmux keeps the machine-shared `bmad-loop-ctl` byte-identically — the - shared session is correct there and every pinned tmux argv depends on it. +@pytest.mark.parametrize("namespaced", [False, True]) +def test_ctl_session_for_is_the_fixed_name_on_every_transport(tmp_path, monkeypatch, namespaced): + """psmux keys its one-server-per-name guard on the data root as well as the + name (psmux/psmux#599, fixed in 3.4.0), so each project's registry holds a + `bmad-loop-ctl` of its own and the per-registry digest earlier releases + minted is retired (#733): the name moves with neither the project, the + state root nor an honoured registry root. tmux keeps the machine-shared + name it always had. - Ablate the `has_registry_namespace()` arm (suffix always) and this fails.""" - assert runs.ctl_session_for(tmp_path, _NamespaceStub(False)) == runs.CTL_SESSION - - -def test_ctl_session_for_carries_the_registry_identity(tmp_path, monkeypatch): - """On a namespacing transport the name is per REGISTRY, because psmux's - duplicate-server mutex is keyed on the session name alone, machine-wide - (`Local\\psmux-session-{name}`, source-read at v3.3.8): a fixed name lets - only one registry on the machine hold a control session, and the second - project's create is rejected as a duplicate server (measured: rc 1). Both - axes of the registry key must move the name — a project-only tag would - recreate the collision for one project under two state roots. - - Ablate the suffix (return the fixed name always) and every assertion but - the stability one fails; key the suffix on `project_tag` alone and the - state-root case fails.""" - mux = _NamespaceStub(True) + Ablate (restore the digest arm for a namespacing transport) and the + namespaced case fails.""" + mux = _NamespaceStub(namespaced) a, b = tmp_path / "proj-a", tmp_path / "proj-b" a.mkdir() b.mkdir() - - name_a = runs.ctl_session_for(a, mux) - name_b = runs.ctl_session_for(b, mux) - assert name_a.startswith(runs.CTL_SESSION + "-") and name_b.startswith(runs.CTL_SESSION + "-") - assert name_a != name_b # two projects, two registries, two names - assert runs.ctl_session_for(a, mux) == name_a # stable per registry - - # ...and the OTHER axis of the registry key: same project, moved state root + names = {runs.ctl_session_for(a, mux), runs.ctl_session_for(b, mux)} monkeypatch.setenv(envvars.STATE_DIR, str(tmp_path / "S2")) - assert runs.ctl_session_for(a, mux) != name_a - - -def test_ctl_session_for_converges_across_spellings_of_one_root(tmp_path, monkeypatch): - """Two spellings of one state root reach ONE physical registry (the OS - resolves both to the same files; psmux keeps the spelling only while - constructing those paths — src/paths.rs:79, source-read at v3.3.8), so - they must mint ONE control-session name: an as-spelled digest gave the - same registry two ctl sessions, each blind to the other's parked windows. - - Ablate the `.resolve()` in ctl_session_for and this fails.""" - mux = _NamespaceStub(True) - project = tmp_path / "proj" - project.mkdir() - (tmp_path / "state").mkdir() - (tmp_path / "alias").mkdir() - plain = str(tmp_path / "state") - detour = str(tmp_path / "alias" / ".." / "state") - assert plain != detour # the premise: two spellings, not one - - monkeypatch.setenv(envvars.STATE_DIR, plain) - name_plain = runs.ctl_session_for(project, mux) - monkeypatch.setenv(envvars.STATE_DIR, detour) - name_detour = runs.ctl_session_for(project, mux) - - assert name_plain.startswith(runs.CTL_SESSION + "-") - assert name_plain == name_detour - - -def test_ctl_session_for_degrades_to_the_fixed_name(tmp_path, monkeypatch): - """The underivable arm runs on the transport's shared default registry — - the one place psmux is in the tmux-shaped world where a shared session - scoped by window tags is correct, and where a pre-#537 legacy ctl session - under the fixed name may exist to be reused rather than collided with.""" - monkeypatch.setenv(envvars.STATE_DIR, "relative-root") - assert runs.ctl_session_for(tmp_path, _NamespaceStub(True)) == runs.CTL_SESSION + names.add(runs.ctl_session_for(a, mux)) + monkeypatch.setenv(runs.PSMUX_DATA_DIR, str(tmp_path / "pinned")) + names.add(runs.ctl_session_for(a, mux)) + assert names == {runs.CTL_SESSION} def test_is_ctl_session_name_shapes(): - """Exactly the shapes ctl_session_for can mint — the fixed name and a - 16-hex suffix. An arbitrary suffix is NOT a control session: it is the + """Exactly the shapes a release has minted — the fixed name, and the 16-hex + suffix earlier releases minted on psmux, which a control session started + before the upgrade still carries (#733). An arbitrary suffix is NOT a control session: it is the agent session of a run an older release accepted (`--run-id ctl-foo`), and reading it as a control session made it unreachable by stop and the prune both. Ablate the 16-hex narrowing (back to any `-` suffix) and the @@ -8003,30 +7914,6 @@ def test_run_id_reservation_stops_at_the_ctl_shape(near_miss): assert runs.is_valid_run_id(near_miss) -def test_ctl_session_for_folds_case_only_where_the_filesystem_does(tmp_path, monkeypatch): - """Two case spellings of a NOT-YET-created state root: `resolve()` can - return stored case only for a path that exists, and the registry root - usually does not exist at name time — so the digest folds case itself, - via `os.path.normcase`. On Windows both spellings land in ONE physical - registry (the `.port` files open case-insensitively), so they must mint - one name; on POSIX case is significant — two case spellings ARE two - registries and must keep two names. - - Ablate the normcase and the win32 arm fails; replace it with an - unconditional `str.lower` and the POSIX arm fails.""" - mux = _NamespaceStub(True) - project = tmp_path / "proj" - project.mkdir() - monkeypatch.setenv(envvars.STATE_DIR, str(tmp_path / "MiXeD-State")) # never created - name_mixed = runs.ctl_session_for(project, mux) - monkeypatch.setenv(envvars.STATE_DIR, str(tmp_path / "mixed-state")) # never created - name_lower = runs.ctl_session_for(project, mux) - if sys.platform == "win32": - assert name_mixed == name_lower - else: - assert name_mixed != name_lower - - def test_kill_session_never_addresses_a_control_session_alias(): """An id that aliases a control session (`ctl`, `ctl-<16 hex>`, folded) can only address the control plane, so its kill is skipped — reachable @@ -8127,12 +8014,8 @@ def test_live_session_may_be_ours_compares_names_the_transports_way(tmp_path, mo # psmux shape: the transport folds, so the case-variant IS the control session monkeypatch.setattr(runs, "get_multiplexer", lambda: _LivenessMux(sessions, fold=True)) assert not runs.live_session_may_be_ours(tmp_path, "CTL") - # this project's derived name is also the control session's - monkeypatch.setattr( - runs, "ctl_session_for", lambda project, mux=None: "bmad-loop-ctl-aaaabbbbccccdddd" - ) - assert not runs.live_session_may_be_ours(tmp_path, "ctl-aaaabbbbccccdddd") - # ...while an OTHER digest in this registry is still not the control session + # ...while a digest-shaped name is not THE control session this process + # addresses, even where an earlier release's control session holds it assert runs.live_session_may_be_ours(tmp_path, "ctl-0123456789abcdef") @@ -8203,13 +8086,10 @@ def test_live_session_may_be_ours_reads_a_successful_listing_as_the_whole_truth( """The accepted ceiling of #732, pinned so it cannot be closed by accident. The two tests above cover a listing that *fails*. This one covers the - listing that succeeds and is wrong: psmux reaps a live session's registry - entry on any `has-session` whose 500 ms connect does not land, and until the - server's registry maintenance re-writes it `ls` exits 0 with that session - simply missing — no error, no stderr. (That maintenance runs on a nominal - 5 s check in the server's own loop, so the window has no hard bound; 1.7 s - was one measured sample.) The guard therefore cannot tell "absent" from - "omitted", and removal proceeds. That is the documented trade, not an + listing that answers and is wrong: a process whose PATH lacks the + multiplexer binary reads every session as absent, with no error and no + stderr. The guard therefore cannot tell "absent" from "omitted", and + removal proceeds. That is the documented trade, not an oversight; a change that makes an empty or short listing block instead has to delete this test and read why. @@ -8223,8 +8103,8 @@ def test_live_session_may_be_ours_reads_a_successful_listing_as_the_whole_truth( monkeypatch.setattr(runs, "get_multiplexer", lambda: _LivenessMux([])) assert not runs.live_session_may_be_ours(tmp_path, "20260826-000000-run1") - # Non-empty and still omitting it: a reap takes one session's entry, not the - # registry, so "the listing came back with rows in it" is no reassurance either. + # Non-empty and still omitting it: "the listing came back with rows in it" is + # no proof that it names every live session either. monkeypatch.setattr(runs, "get_multiplexer", lambda: _LivenessMux([runs.session_name("other")])) assert not runs.live_session_may_be_ours(tmp_path, "20260826-000000-run1") @@ -8251,15 +8131,14 @@ def test_prune_sessions_claims_a_historical_ctl_prefixed_session(tmp_path, monke assert runs.prune_sessions(tmp_path, dry_run=True) == (["ctl-foo"], [], set()) -def test_no_valid_run_id_can_mint_the_control_session_name(tmp_path): - """The reviewer's reproduction, pinned end to end: read the live per-registry - control-session name, replay its suffix as a --run-id, and the id is - refused before `session_name` can alias the two session types. Covers the - fixed name too — `--run-id ctl` minted `bmad-loop-ctl` itself, on every - transport.""" - project = tmp_path / "proj" - project.mkdir() - ctl = runs.ctl_session_for(project, _NamespaceStub(True)) +def test_no_valid_run_id_can_mint_the_control_session_name(): + """The reviewer's reproduction, pinned end to end: take a per-registry + control-session name an earlier release minted, replay its suffix as a + --run-id, and the id is refused before `session_name` can alias the two + session types. Covers the fixed name too — `--run-id ctl` minted + `bmad-loop-ctl` itself, on every transport; the reservation does not + depend on the digest that #733 retired.""" + ctl = runs.CTL_SESSION + "-0123456789abcdef" colliding_id = ctl[len("bmad-loop-") :] assert runs.session_name(colliding_id) == ctl # the alias, were the id valid assert not runs.is_valid_run_id(colliding_id) diff --git a/tests/test_tui_launch.py b/tests/test_tui_launch.py index df8d2bb2..f6312bce 100644 --- a/tests/test_tui_launch.py +++ b/tests/test_tui_launch.py @@ -967,16 +967,18 @@ def test_current_return_target_none_on_empty_pane(monkeypatch): assert launch.current_return_target() is None -def test_start_detached_uses_the_per_registry_ctl_name(tmp_path: Path, monkeypatch): - """On a namespacing transport the launcher creates and parks into the - per-registry control session (runs.ctl_session_for), never the fixed name: - psmux's duplicate-server mutex is keyed on the session name machine-wide, - so a second project minting the fixed `bmad-loop-ctl` is rejected as a - duplicate and its launch fails. The tmux half — the fixed name, - byte-identical argv — is pinned by test_start_run_detached_argv. - - Ablate `runs.ctl_session_for` at `_ensure_ctl_session` / the parked-window - call (hardcode CTL_SESSION) and this fails.""" +def test_start_detached_uses_the_fixed_ctl_name_on_a_namespacing_transport( + tmp_path: Path, monkeypatch +): + """On a namespacing transport the launcher creates and parks into the fixed + `bmad-loop-ctl`, in the project's own registry: psmux keys its + one-server-per-name guard on the data root too (psmux/psmux#599), so every + project's registry can hold one, and the per-registry digest earlier + releases minted is retired (#733). The tmux half — byte-identical argv — + is pinned by test_start_run_detached_argv. + + Ablate (restore a per-registry name in `runs.ctl_session_for`) and this + fails.""" class _NamespacedStub: def __init__(self): @@ -1007,10 +1009,8 @@ def set_window_option(self, window, option, value): monkeypatch.setattr(launch, "mux_usable", lambda _m: True) assert launch.start_detached(tmp_path, ["run"], "RID", "run") == "@7" - expected = runs.ctl_session_for(tmp_path, stub) - assert expected.startswith(runs.CTL_SESSION + "-") - assert stub.created == [expected] - assert stub.parked == [(expected, "run-RID")] + assert stub.created == [runs.CTL_SESSION] + assert stub.parked == [(runs.CTL_SESSION, "run-RID")] @pytest.mark.parametrize( @@ -2207,16 +2207,17 @@ def capture_pane(self, win_id): return _PARKED_SCREEN -def test_launch_addresses_the_per_registry_control_session(monkeypatch, tmp_path: Path): +def test_launch_addresses_the_resolved_control_session(monkeypatch, tmp_path: Path): """Every launch-layer read of the control session resolves its name through `runs.ctl_session_for`, never the `CTL_SESSION` constant. - On a namespacing transport the name carries the registry digest, so a site - still spelling the constant addresses a session that does not exist there: - `list_windows` answers empty and attach reports "nothing to attach", while - the post-kill listing reads every candidate as removed — cleanup claims - windows it never closed. All three fail silently, which is why the constant - survived at these sites at all. + The two agree today on every bundled transport, so the resolver is pinned + to a distinct name here: a site spelling the constant would address a + session that does not exist wherever they part — `list_windows` answers + empty and attach reports "nothing to attach", while the post-kill listing + reads every candidate as removed, so cleanup claims windows it never + closed. All three fail silently, which is why the constant survived at + these sites once already. Ablate any one of `ctl_window_id`'s listing, `ctl_target`'s token, or `prune_ctl_windows`' post-kill listing back to `runs.CTL_SESSION` and the @@ -2226,11 +2227,8 @@ def test_launch_addresses_the_per_registry_control_session(monkeypatch, tmp_path mine = runs.project_tag(tmp_path) mux = _NamespacedMux([("@2", "run-20260101-000000-dead", mine)]) monkeypatch.setattr(launch, "get_multiplexer", lambda: mux) - - expected = runs.ctl_session_for(tmp_path, mux) - # premise: the digest name is what this project's control session is called, - # and it is NOT the constant — without this the assertion below is vacuous - assert expected.startswith(runs.CTL_SESSION + "-") and expected != runs.CTL_SESSION + expected = "bmad-loop-ctl-resolved" + monkeypatch.setattr(runs, "ctl_session_for", lambda _p, _m=None: expected) assert launch.ctl_window_id(tmp_path, "20260101-000000-dead") == "@2" assert launch.ctl_target(tmp_path) == f"={expected}"