From 91ad537055313681d324997a31fa40a55d6b1a80 Mon Sep 17 00:00:00 2001 From: pbean Date: Mon, 13 Jul 2026 22:00:34 -0700 Subject: [PATCH 1/2] feat(mux): herdr TUI-launch surface Complete the herdr backend's tui/launch.py surface (PR 2 of the herdr series): parked orchestrator windows via a typed 'exec sh -c' recipe with a per-window return file the trailer consumes, one _parse_target helper for tmux-style '=session[:window]' targets across select/kill/option/ attach/switch methods (window-option sidecar keys normalized to the native pane id), list_windows tmux-field mapping (window_id/window_name/ @options via a tab-label join), finalized attach_target_argv (terminal attach outside herdr, fire-and-forget tab focus inside), and switch_client via tab focus. Seam-honesty migration: the TUI/CLI attach flows and in_ctl_session now ask the seam (current_pane_id/current_session) instead of sniffing TMUX; BaseTmuxBackend._display_message gains the TMUX guard that keeps 'not inside' honest (behavior-preserving at every call site, pinned by tests). Unit tests extend FakeHerdr with tabs/focus/active_tab_id; a new HAVE_HERDR-gated live test drives launch.start_detached(validate) end-to-end (park -> banner -> return-file -> unpark -> workspace survives). Full suite green with and without TMUX in the env; win32-sim (forced herdr, no binary) green on all touched files. --- CHANGELOG.md | 18 +- docs/ROADMAP.md | 2 +- docs/porting-to-a-new-os.md | 9 +- src/bmad_loop/adapters/herdr_backend.py | 441 +++++++++++++++++++++--- src/bmad_loop/adapters/tmux_base.py | 8 +- src/bmad_loop/cli.py | 10 +- src/bmad_loop/tui/app.py | 17 +- src/bmad_loop/tui/launch.py | 7 +- tests/test_herdr_backend.py | 307 +++++++++++++++-- tests/test_herdr_integration.py | 61 ++++ tests/test_tui_launch.py | 42 ++- 11 files changed, 829 insertions(+), 93 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d138838f..60089553 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,19 @@ breaking changes may land in a minor release. forced selection. A tmux-less POSIX host still selects `TmuxMultiplexer` and reports it unavailable, exactly as before. +- **Herdr TUI-launch surface.** The herdr backend now covers everything `tui/launch.py` drives: + parked orchestrator windows (a typed `exec sh -c '; banner; read; trailer'` recipe, + tmux-identical from the operator's seat, with the return-to-origin target mirrored into a + per-window return file the trailer consumes), tmux-style `=session[:window]` target parsing + across select/kill/option/attach methods, `list_windows` tmux-field mapping + (`window_id`/`window_name`/`@option`), finalized `attach_target_argv` (blocking + `terminal attach` outside herdr, fire-and-forget `tab focus` inside), and `switch_client` + via `tab focus`. The TUI/CLI attach flows and `in_ctl_session` now ask the seam + (`current_pane_id()`/`current_session()`) instead of sniffing `TMUX`, so they work under any + backend; the tmux backend's `current_*` gained the TMUX guard that keeps "not inside" honest. + Remaining herdr degradations: `detach_client` stays a no-op (the post-exit detach return rides + pane-close ending `terminal attach`; only the mid-process hand-back degrades to staying + attached), and `switch_client` has no last-client fallback. - **Herdr multiplexer backend (opt-in, non-tmux-family).** A new `herdr` transport implements the `TerminalMultiplexer` seam _fresh_ (subclassing nothing) over [herdr](https://herdr.dev)'s cross-platform workspace/tab/pane model — Unix socket on POSIX, @@ -65,9 +78,8 @@ breaking changes may land in a minor release. session/window options (herdr has none; writes lock-guarded via a new cross-platform `platform_util.file_lock`), lazy server autostart, and a protocol-version guard (fail-below / warn-above). Landed as one registration line plus one sanctioned `probe.py` gate - fix — zero portability-guard allowlist changes. The TUI-launch surface (parked windows, - attach/detach, full window options) is a follow-up PR; until then those degrade to raises / no-ops - (see the backend module docstring's degradation ledger). + fix — zero portability-guard allowlist changes. The TUI-launch surface landed as a second PR + (previous entry); the remaining degradations are in the backend module docstring's ledger. - **Stories mode — a second planning pipeline that drives the loop off a typed `stories.yaml` (folder+id dispatch) instead of `sprint-status.yaml`.** Opt in with `[stories] source = "stories"` + `spec_folder`, or per run with `bmad-loop run --spec ` (overrides policy); `--story` then filters by story id. Each entry dispatches by folder + id — the dev skill creates-or-resumes the story spec at `/stories/-.md` and the orchestrator reads that id-keyed path back deterministically (no shared board to line-edit, no result-artifact mtime-scan). Strictly linear schedule (list order, no `depends_on`); `bmad-loop run --dry-run --spec ` and `bmad-loop status` print the board (id · live disk state · checkpoint markers · title). Sprint mode is unchanged and remains the default. Requires a `bmad-dev-auto` new enough for folder+id dispatch — the run preflight checks and remediates. - **Per-story human checkpoints (stories mode).** Independent `spec_checkpoint` (pause before code to review the plan — dev halts at `ready-for-dev`; approve to implement, or request a replan that resets the spec to `draft`) and `done_checkpoint` (pause after the story commits, skipped when it is the last story); both additive to `gates.mode`. A blocked story escalates + resolves as in sprint mode, with a pre-planning-halt sentinel auto-deleted (a copy preserved under the run dir) on re-arm. - **TUI human-in-the-loop surface for stories mode.** The sprint tree is replaced by a stories board (id · live disk state · spec/done checkpoint markers · title) when a stories-mode run is selected; paused runs carry a per-run pause-kind badge and the run list shows a global _⚑ N need attention_ count; `p` opens the stage-appropriate viewer — plan-checkpoint spec review (Approve & resume / Request replan), story-checkpoint summary card (Continue / Stop), escalation with story context (Resolve / Re-arm & resume), and a gate spec viewer that the existing spec-approval/epic pauses reuse. The start-run modal gains a source select + spec-folder field with a live schedule preview. Every TUI action calls the same code paths as the CLI. diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 05ec84ca..037f5ebc 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -9,7 +9,7 @@ Status legend: **planned** (agreed, not started) · **exploring** (shape still o ## Native Windows multiplexer backend -**Status:** planned · **Foundation:** the full platform-seam series landed (multiplexer registry + `BaseTmuxBackend` + `ProcessHost` + hook interpreter + validate preflight, v0.7.6; availability-aware backend selection + `bmad-loop mux`, #87; original seam v0.7.0) · a first non-tmux-family, native-Windows-capable backend — **herdr** — now ships for the engine run path (TUI-launch surface follows in a second PR) +**Status:** planned · **Foundation:** the full platform-seam series landed (multiplexer registry + `BaseTmuxBackend` + `ProcessHost` + hook interpreter + validate preflight, v0.7.6; availability-aware backend selection + `bmad-loop mux`, #87; original seam v0.7.0) · a first non-tmux-family, native-Windows-capable backend — **herdr** — now ships end-to-end on POSIX (engine run path + TUI-launch surface; the win32 `agent.start` launch path is the remaining follow-up) The orchestrator no longer fuses tmux into the engine. All session/window/pane operations go through a single `TerminalMultiplexer` ABC (`src/bmad_loop/adapters/multiplexer.py`), diff --git a/docs/porting-to-a-new-os.md b/docs/porting-to-a-new-os.md index 65e33a96..ee966405 100644 --- a/docs/porting-to-a-new-os.md +++ b/docs/porting-to-a-new-os.md @@ -95,10 +95,11 @@ version, and the current selection. (The result is cached — see session is a herdr **workspace** (label == session name), a window is a **tab** (its `root_pane.pane_id` is the native window id), and the launched command runs via a typed `exec ` so process-exit stays tmux-identical window death. - Where herdr has no analogue for a contract method — options, `pipe_pane`, parked - windows, detach — it degrades honestly (a JSON sidecar for options, a polling - tee for `pipe_pane`, a raise / no-op for the rest); that **degradation ledger** - is the module docstring, and it is the template for what "implement fresh" costs + Where herdr has no analogue for a contract method — options, `pipe_pane`, + the parked-window return hop, detach — it emulates or degrades honestly (a + JSON sidecar for options, a polling tee for `pipe_pane`, a per-window return + file for the parked trailer, a no-op detach); that **degradation ledger** is + the module docstring, and it is the template for what "implement fresh" costs in practice. `available()` gates whether the backend is usable on the current host (e.g. its diff --git a/src/bmad_loop/adapters/herdr_backend.py b/src/bmad_loop/adapters/herdr_backend.py index 191b33c5..49bfa5fe 100644 --- a/src/bmad_loop/adapters/herdr_backend.py +++ b/src/bmad_loop/adapters/herdr_backend.py @@ -24,7 +24,7 @@ / ``_herdr`` / ``_herdr_json`` primitives plus the server lifecycle), so a future socket transport can replace it without touching :class:`HerdrMultiplexer`. -**Degradation ledger (PR 1 scope — the run path; the TUI-launch surface is PR 2):** +**Degradation ledger (PR 1 = run path; PR 2 added the TUI-launch surface):** - ``pipe_pane`` has no herdr ``pipe-pane``/tee to hand off to, so it runs a per-window :class:`_PanePoller` daemon that snapshots ``pane read`` into the @@ -32,21 +32,41 @@ ``revision`` is unusable, it stays 0). This drives the two log consumers a tmux tee would: ``generic._log_activity_key`` re-arms the dev-stall grace on log growth, and ``probe`` finds completion markers in the log. -- ``new_parked_window`` **raises** ``HerdrError`` — only ``tui/launch.py`` calls - it (PR 2). -- ``attach_target_argv`` returns ``["herdr", "terminal", "attach", ]`` - — it strips a tmux-style ``=`` target prefix and resolves the window's pane id to - its ``terminal_id`` via ``pane get``, raising ``HerdrError`` with operator - guidance when no terminal can be resolved. Full ``=session:window`` target - parsing and ``switch_client`` are PR 2 (the TUI-launch surface). -- ``detach_client`` is a no-op — herdr detach is a keybinding, with no CLI verb. +- ``new_parked_window`` types a POSIX ``exec sh -c '; ec=$?; echo + ; read -r; '`` recipe into a fresh tab, tmux-identical from + the operator's seat. The tmux trailer reads the return option live via + ``show-options``; herdr window options live in OUR sidecar, which a one-line + ``sh`` trailer can't query — so the option methods mirror the parked window's + return option into a per-window **return file** the trailer ``cat``\\ s + (resolved to a *tab id* at write time, keeping the trailer a dumb + ``herdr tab focus``). POSIX-only, like the ``exec`` launch (pwsh dialect is + the Windows follow-up). +- ``attach_target_argv`` accepts both target families (native pane id and the + tmux-style ``=session[:window]`` specs ``tui/launch.py`` builds — see + ``_parse_target``): outside herdr it returns ``["herdr", "terminal", + "attach", ]`` (which blocks, and exits when the pane closes); + inside a herdr pane it returns the fire-and-forget ``["herdr", "tab", + "focus", ]`` — the switch-client move, mirroring tmux's in-``TMUX`` + branch. Raises ``HerdrError`` with operator guidance when unresolvable. +- ``switch_client`` is a ``tab focus`` on the target's tab (focusing a tab in + another workspace flips workspace focus too — verified 0.7.3). herdr has no + "last client" concept, so ``last_fallback`` has nothing to fall back to and + a failed switch is honestly ``False``. +- ``detach_client`` is a no-op — herdr detach is a keybinding, with no CLI + verb. Consequence: the *post-exit* detach return is full-fidelity anyway + (ending the parked source closes the pane, which ends a blocking ``terminal + attach`` client), but the *mid-process* ``RETURN_DETACH`` hand-back + (``launch.return_attached_client`` while a plain-terminal client watches) + degrades to "stay attached until the parked window closes". - Session/window **options** have no native herdr equivalent, so they live in a cross-process **sidecar** JSON (``~/.bmad-loop/herdr-state.json``, override ``BMAD_LOOP_HERDR_STATE``), written via :func:`platform_util.atomic_replace` with every read-modify-write cycle serialized by an OS advisory lock on a sibling ``.lock`` file (:func:`platform_util.file_lock`) — the write serialization the tmux server gave ``set-option`` for free; entries for a - workspace that is gone are pruned on the next enumeration. + workspace that is gone are pruned on the next enumeration. Window-option keys + are **normalized to the native pane id** (``_parse_target``) so a write by + ``=session:name`` target and a read by native id agree. - ``new_session`` geometry (cols/lines) is **advisory** — herdr exposes no absolute headless resize; a detached pane takes an attaching client's size. - Protocol policy: fail below :data:`SUPPORTED_PROTOCOL`, warn once above. @@ -89,6 +109,18 @@ # current_* accessors know this process is running inside a herdr pane. _HERDR_ENV_MARKER = "HERDR_ENV" +# Per-window option value (vs a pane id) telling the parked trailer to detach +# the client rather than switch it — the same protocol constant the tmux family +# uses (tmux_base.PARKED_RETURN_DETACH); duplicated so this backend stays free +# of tmux imports. +PARKED_RETURN_DETACH = "detach" + +# Private sidecar window-entry key recording which user option is a parked +# window's return option (new_parked_window writes it; set/unset_window_option +# consult it to mirror that option into the window's return file). "~"-prefixed +# so it can never collide with a tmux-style "@" user option. +_PARKED_RETURN_KEY = "~parked_return_opt" + class HerdrError(MultiplexerError): """A herdr transport op failed — the seam type, so call sites catch it via @@ -168,6 +200,66 @@ def _save_state(state: dict) -> None: raise +# ------------------------------------------------------------- return files +# +# A parked window's trailing shell must read the CURRENT value of its return +# option at park-exit time. tmux trailers run `show-options` against the live +# server; herdr window options live in the sidecar JSON above, which a one-line +# sh trailer cannot sensibly parse — so the option methods mirror the parked +# return option into a tiny per-window file the trailer `cat`s. The path is +# deterministic from the sidecar location, so the window creator (one process), +# the attach-time writer (another), and the in-window trailer (a third) all +# agree with no coordination. + + +def _return_file(pane_id: str) -> Path: + # ':' is not a legal filename character on Windows, hence the '-' mapping. + state = _state_path() + return state.with_name(f"herdr-return-{pane_id.replace(':', '-')}") + + +def _write_return_file(pane_id: str, content: str) -> None: + """Best-effort: the option write already succeeded, and the mirror only + serves the trailer — a failure here must not fail the (sentinel) caller.""" + path = _return_file(pane_id) + try: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(content + "\n", encoding="utf-8") + except OSError: + pass + + +def _remove_return_file(pane_id: str) -> None: + try: + _return_file(pane_id).unlink() + except OSError: + pass + + +def _parked_source(argv: list[str], pane_id: str) -> str: + """The POSIX sh source a parked window runs. Typed via ``pane run``, so it + must be a single line; it passes through the pane's default shell before + ``exec sh -c`` takes over, so it sticks to quoting every POSIX family + member AND fish accept (single-quoted payloads with the shlex ``'"'"'`` + concatenation dance). + + After the park, the trailer hands an attached client back to its origin: + the return file holds a tab id to focus (one ``tab focus`` also flips + workspace focus — verified 0.7.3) or :data:`PARKED_RETURN_DETACH`, where + doing NOTHING is correct — ending the source closes the pane, and a + ``herdr terminal attach`` client exits when its pane closes (verified + 0.7.3).""" + ret = shlex.quote(str(_return_file(pane_id))) + return ( + f"{shlex.join(argv)}; ec=$?; " + f'echo "[bmad-loop exited $ec — press enter]"; ' + "read -r; " + f"ret=$(cat {ret} 2>/dev/null); rm -f {ret}; " + f'if [ -n "$ret" ] && [ "$ret" != {PARKED_RETURN_DETACH} ]; then ' + f'herdr tab focus "$ret" >/dev/null 2>&1 || true; fi' + ) + + # ----------------------------------------------------------------- transport # # The ONE place herdr is spawned. Everything protocol-specific about the CLI wire @@ -516,8 +608,32 @@ def _list_panes_strict(self) -> list[dict]: raise HerdrError(f"herdr pane list failed: {proc.stderr.strip()}") return _envelope_items(proc.stdout, "panes", strict=True) - def _workspace_id(self, label: str, *, strict: bool) -> str | None: - """First-match resolution of a session name to a workspace id. herdr + def _list_tabs_strict(self) -> list[dict]: + """Every tab across all workspaces; raises on failure. Never + ``tab list --workspace `` — that RAISES ``workspace_not_found`` + (the pane-list trap again); enumerate all tabs and filter in-process.""" + try: + proc = self._client._run(["tab", "list"], check=False) + except (subprocess.TimeoutExpired, OSError) as exc: + raise HerdrError(f"herdr tab list failed: {exc}") from exc + if proc.returncode != 0: + raise HerdrError(f"herdr tab list failed: {proc.stderr.strip()}") + return _envelope_items(proc.stdout, "tabs", strict=True) + + def _list_tabs_tolerant(self) -> list[dict] | None: + """Every tab, or None when the query could not be answered.""" + if not shutil.which("herdr"): + return None + try: + proc = self._client._run(["tab", "list"], check=False) + except (subprocess.SubprocessError, OSError): + return None + if proc.returncode != 0: + return None + return _envelope_items(proc.stdout, "tabs", strict=False) + + def _workspace_row(self, label: str, *, strict: bool) -> dict | None: + """First-match resolution of a session name to its workspace row. herdr allows DUPLICATE labels (tmux session names are unique; herdr's are not), so callers must tolerate more than one and take the first.""" workspaces = self._list_workspaces_strict() if strict else self._list_workspaces_tolerant() @@ -525,10 +641,68 @@ def _workspace_id(self, label: str, *, strict: bool) -> str | None: return None for ws in workspaces: if ws.get("label") == label: - wid = ws.get("workspace_id") - return wid if isinstance(wid, str) else None + return ws if isinstance(ws.get("workspace_id"), str) else None + return None + + def _workspace_id(self, label: str, *, strict: bool) -> str | None: + row = self._workspace_row(label, strict=strict) + return row["workspace_id"] if row is not None else None + + def _tab_root_pane(self, tab_id: str, *, strict: bool) -> str | None: + """The tab's first pane — bmad-loop windows are single-pane tabs, so this + is the pane that IS the window.""" + try: + panes = self._list_panes_strict() + except MultiplexerError: + if strict: + raise + return None + for pane in panes: + if pane.get("tab_id") == tab_id and isinstance(pane.get("pane_id"), str): + return pane["pane_id"] return None + def _parse_target(self, target: str, *, strict: bool) -> str | None: + """Resolve any window target to a native pane id. + + Callers hand this two families: a tmux-style ``=session[:window]`` spec + (``tui/launch.py`` builds these) and our own native pane id. The ``=`` + prefix is the discriminator — native ids contain ``:`` (``w1:p1``) but + never lead with ``=``. A native id passes through untouched, with no + server round-trip. For ``=…`` targets: session → workspace (first-match + label), a window name → the tab with that label, no window → the + session-level tab (see :func:`_session_level_tab`), then that tab's + pane. ``strict=True`` raises :class:`HerdrError` when the target cannot + be resolved; ``strict=False`` returns None (the sentinel callers' quiet + failure).""" + if not target.startswith("="): + return target + session, _, window = target[1:].partition(":") + row = self._workspace_row(session, strict=strict) + if row is None: + if strict: + raise HerdrError(f"herdr workspace for session {session!r} not found") + return None + wid = row["workspace_id"] + tabs = self._list_tabs_strict() if strict else self._list_tabs_tolerant() + tabs = [ + t + for t in tabs or [] + if t.get("workspace_id") == wid and isinstance(t.get("tab_id"), str) + ] + if window: + tab = next((t for t in tabs if t.get("label") == window), None) + else: + tab = _session_level_tab(tabs, row.get("active_tab_id")) + if tab is None: + if strict: + raise HerdrError(f"herdr target {target!r} matches no tab") + return None + pane_id = self._tab_root_pane(tab["tab_id"], strict=strict) + if pane_id is None and strict: + raise HerdrError(f"herdr tab {tab['tab_id']!r} for target {target!r} has no pane") + return pane_id + # ------------------------------------------------------------ sessions def has_session(self, name: str) -> bool: @@ -567,6 +741,7 @@ def kill_session(self, name: str) -> None: wid = None if wid is not None: self._stop_pollers_for_workspace(wid) + _drop_windows_for_workspace(wid) _drop_state("sessions", name) def list_sessions(self) -> list[str]: @@ -654,11 +829,37 @@ def _launch(self, pane_id: str, argv: list[str]) -> None: def new_parked_window( self, session: str, name: str, cwd: Path, argv: list[str], return_opt: str ) -> str: - # The parked/return-to-origin recipe is the TUI-launch surface (PR 2); - # only tui/launch.py calls this. Fail loudly rather than half-implement. - raise HerdrError( - "herdr new_parked_window (TUI launch surface) is not implemented yet — see PR 2" + # A fresh tab whose typed `exec sh -c ''` runs argv, echoes the + # exit banner, parks on a blocking read (the exit status stays + # inspectable), and finally hands an attached client back to its origin + # via the per-window return file (see _parked_source / the module + # docstring's ledger). exec replaces the shell, so finishing the source + # closes the pane — which is also what ends a watching `terminal + # attach` client. + self._client.ensure_server() + wid = self._workspace_id(session, strict=True) + if wid is None: + raise HerdrError(f"herdr workspace for session {session!r} not found") + result = self._client._herdr_json( + "tab", "create", "--workspace", wid, "--label", name, "--cwd", str(cwd), "--no-focus" + ) + pane_id = _root_pane_id(result) + if pane_id is None: + raise HerdrError(f"herdr tab create did not return a root pane id: {result!r}") + # Record which option this window's trailer consumes BEFORE typing the + # recipe, so a set_window_option racing the launch already mirrors into + # the return file. + try: + with _state_lock(): + state = _load_state() + state["windows"].setdefault(pane_id, {})[_PARKED_RETURN_KEY] = return_opt + _save_state(state) + except OSError as exc: + raise HerdrError(f"herdr sidecar write failed: {exc}") from exc + self._client._herdr( + "pane", "run", pane_id, "exec sh -c " + shlex.quote(_parked_source(argv, pane_id)) ) + return pane_id def list_window_ids(self, session: str) -> list[str]: # The engine's liveness probe: [] means "no windows", and a transport @@ -676,9 +877,12 @@ def list_window_ids(self, session: str) -> list[str]: ] def list_windows(self, session: str, fields: list[str]) -> list[tuple[str, ...]]: - # Best-effort metadata (sentinel [] on any failure). Fields are looked up - # directly on each pane dict by herdr-native key; full tmux-field mapping - # for tui/launch.py is PR 2. + # Best-effort metadata (sentinel [] on any failure). The tmux format + # fields tui/launch.py asks for are mapped onto the herdr object model + # (window == tab, native window id == the tab's root pane id): + # window_id/pane_id -> pane_id, window_name -> the owning tab's label, + # session_name -> the session, "@..." user options -> the sidecar. + # Anything unmapped falls back to the pane dict's own herdr-native key. wid = self._workspace_id(session, strict=False) if wid is None: return [] @@ -686,11 +890,40 @@ def list_windows(self, session: str, fields: list[str]) -> list[tuple[str, ...]] panes = self._list_panes_strict() except MultiplexerError: return [] + tab_labels: dict[str, str] = {} + if "window_name" in fields: + tabs = self._list_tabs_tolerant() + if tabs is None: + return [] # can't answer a requested field -> the [] sentinel + for tab in tabs: + if isinstance(tab.get("tab_id"), str): + label = tab.get("label") + tab_labels[tab["tab_id"]] = label if isinstance(label, str) else "" + options: dict = {} + if any(field.startswith("@") for field in fields): + options = _load_state()["windows"] rows: list[tuple[str, ...]] = [] for pane in panes: if pane.get("workspace_id") != wid: continue - rows.append(tuple(str(pane.get(field, "")) for field in fields)) + pane_id = pane.get("pane_id") + pane_id = pane_id if isinstance(pane_id, str) else "" + row: list[str] = [] + for field in fields: + if field in ("window_id", "pane_id"): + row.append(pane_id) + elif field == "window_name": + tab_id = pane.get("tab_id") + row.append(tab_labels.get(tab_id, "") if isinstance(tab_id, str) else "") + elif field == "session_name": + row.append(session) + elif field.startswith("@"): + opts = options.get(pane_id, {}) + value = opts.get(field, "") if isinstance(opts, dict) else "" + row.append(value if isinstance(value, str) else "") + else: + row.append(str(pane.get(field, ""))) + rows.append(tuple(row)) return rows def window_alive(self, session: str, window_id: str) -> bool: @@ -721,21 +954,28 @@ def kill_window(self, target: str) -> None: # Best-effort: `pane close` cascades the now-empty tab closed; tolerate the # pane already being gone (a CLI that crashes on launch races its window # down before teardown — probe.py depends on this tolerance). - self._stop_poller(target) + pane_id = self._parse_target(target, strict=False) + if pane_id is None: + return + self._stop_poller(pane_id) if shutil.which("herdr"): try: - self._client._run(["pane", "close", target], check=False) + self._client._run(["pane", "close", pane_id], check=False) except (subprocess.SubprocessError, OSError): pass - _drop_state("windows", target) + _drop_state("windows", pane_id) + _remove_return_file(pane_id) def select_window(self, target: str) -> None: # Best-effort focus: resolve the pane's tab and focus it (herdr has # `tab focus`, not `pane focus`). Swallow any failure to the no-op sentinel. if not shutil.which("herdr"): return + pane_id = self._parse_target(target, strict=False) + if pane_id is None: + return try: - proc = self._client._run(["pane", "get", target], check=False) + proc = self._client._run(["pane", "get", pane_id], check=False) if proc.returncode != 0: return pane = json.loads(proc.stdout).get("result", {}).get("pane", {}) @@ -747,31 +987,75 @@ def select_window(self, target: str) -> None: def set_window_option(self, target: str, option: str, value: str) -> None: # Best-effort sidecar write (sentinel: swallow OSError to a no-op — a - # lock failure skips the write entirely, never writes unguarded). + # lock failure skips the write entirely, never writes unguarded). Keys + # are normalized to the native pane id, so a write by tmux-style + # "=session:name" target (set_return_pane) and a read by native id + # (return_attached_client) agree. + pane_id = self._parse_target(target, strict=False) + if pane_id is None: + return + parked_opt = None try: with _state_lock(): state = _load_state() - state["windows"].setdefault(target, {})[option] = value + opts = state["windows"].setdefault(pane_id, {}) + opts[option] = value + parked_opt = opts.get(_PARKED_RETURN_KEY) _save_state(state) except OSError: - pass + return + if parked_opt == option: + self._mirror_return_value(pane_id, value) + + def _mirror_return_value(self, pane_id: str, value: str) -> None: + """Mirror a parked window's return-option value into its return file, in + trailer-actionable form: :data:`PARKED_RETURN_DETACH` verbatim; a pane id + is resolved to its TAB id here, where JSON parsing is cheap, so the sh + trailer stays a dumb ``tab focus``. Best-effort — an unresolvable origin + clears the file instead, so a stale target can't yank focus somewhere + wrong later.""" + if value == PARKED_RETURN_DETACH: + _write_return_file(pane_id, value) + return + tab_id: str | None = None + try: + candidate = self._client._herdr_json("pane", "get", value).get("pane", {}) + if isinstance(candidate, dict) and isinstance(candidate.get("tab_id"), str): + tab_id = candidate["tab_id"] or None + except MultiplexerError: + tab_id = None + if tab_id is None: + _remove_return_file(pane_id) + else: + _write_return_file(pane_id, tab_id) def unset_window_option(self, target: str, option: str) -> None: + pane_id = self._parse_target(target, strict=False) + if pane_id is None: + return + parked_opt = None try: with _state_lock(): state = _load_state() - opts = state["windows"].get(target) + opts = state["windows"].get(pane_id) if isinstance(opts, dict) and option in opts: del opts[option] + parked_opt = opts.get(_PARKED_RETURN_KEY) if not opts: - del state["windows"][target] + del state["windows"][pane_id] _save_state(state) except OSError: - pass + return + if parked_opt == option: + _remove_return_file(pane_id) def show_window_option(self, target: str, option: str) -> str: - # "" reads as "unset" — also the failure sentinel. - opts = _load_state()["windows"].get(target, {}) + # "" reads as "unset" — also the failure sentinel (including a "=..." + # target that no longer resolves). Keys normalized like set_window_option. + pane_id = self._parse_target(target, strict=False) + if pane_id is None: + return "" + opts = _load_state()["windows"].get(pane_id, {}) value = opts.get(option, "") if isinstance(opts, dict) else "" return value if isinstance(value, str) else "" @@ -820,17 +1104,30 @@ def send_text(self, window_id: str, text: str) -> None: # ----------------------------------------------------- client / attach def attach_target_argv(self, target: str) -> list[str]: - # Attach a plain terminal to the window's pane via its terminal_id. Strip a - # tmux-style '=' target prefix a caller might pass; full '=session:window' - # parsing is PR 2 (here `target` is our native pane id). - pane_id = target[1:] if target.startswith("=") else target + # Resolve either target family (native pane id or a tmux-style + # "=session[:window]" spec — see _parse_target) to its pane, then: + # - inside a herdr pane (HERDR_ENV): nesting a full attach is the + # wrong move, exactly like attach-inside-tmux — return the + # fire-and-forget `tab focus`, the switch-client equivalent; + # - outside: attach the caller's terminal to the pane's terminal_id + # (`herdr terminal attach` blocks, and exits when the pane closes). try: + pane_id = self._parse_target(target, strict=True) + if pane_id is None: # strict never returns None; belt and braces + raise HerdrError("target resolves to no pane") result = self._client._herdr_json("pane", "get", pane_id) except HerdrError as exc: raise HerdrError( f"cannot resolve a herdr terminal for {target!r} to attach: {exc}" ) from exc - terminal_id = result.get("pane", {}).get("terminal_id") + pane = result.get("pane") + pane = pane if isinstance(pane, dict) else {} + if os.environ.get(_HERDR_ENV_MARKER) == "1": + tab_id = pane.get("tab_id") + if isinstance(tab_id, str) and tab_id: + return ["herdr", "tab", "focus", tab_id] + raise HerdrError(f"herdr pane {pane_id!r} has no tab to focus") + terminal_id = pane.get("terminal_id") if not isinstance(terminal_id, str) or not terminal_id: raise HerdrError(f"herdr pane {pane_id!r} has no terminal to attach") return ["herdr", "terminal", "attach", terminal_id] @@ -868,9 +1165,26 @@ def detach_client(self) -> None: return None def switch_client(self, target: str, last_fallback: bool = False) -> bool: - # Client switching is the TUI-launch surface (PR 2). Until then no switch - # happens, so the honest sentinel is False. - return False + # The herdr "switch client" move is a tab focus: focusing a tab also + # flips workspace focus when it lives elsewhere (verified 0.7.3), so one + # verb covers the whole return-to-origin hop. True iff the focus landed. + # herdr has no "last client" concept, so last_fallback has nothing to + # fall back to and a failed switch is honestly False. + pane_id = self._parse_target(target, strict=False) + if pane_id is None: + return False + try: + pane = self._client._herdr_json("pane", "get", pane_id).get("pane", {}) + except MultiplexerError: + return False + tab_id = pane.get("tab_id") if isinstance(pane, dict) else None + if not isinstance(tab_id, str) or not tab_id: + return False + try: + proc = self._client._run(["tab", "focus", tab_id], check=False) + except (subprocess.SubprocessError, OSError): + return False + return proc.returncode == 0 def available(self) -> bool: # A plain PATH probe — NEVER touches the server (detect_multiplexers @@ -905,6 +1219,29 @@ def _envelope_items(stdout: str, key: str, *, strict: bool) -> list[dict]: return [item for item in items if isinstance(item, dict)] +def _session_level_tab(tabs: list[dict], active_tab_id: object) -> dict | None: + """The tab a session-level (``=session``) target lands on: the workspace's + active tab — except when that is the root shell tab and task tabs exist, + where the NEWEST tab wins. Rationale: ``new_window``/``new_parked_window`` + create tabs ``--no-focus`` (never yanking an attached client's focus), so a + headless agent workspace keeps its root shell active while the window the + caller wants is the newest tab; the ctl session is unaffected because + ``attach_plan`` runs ``select_window`` (a ``tab focus``) first, making the + intended window the active one.""" + if not tabs: + return None + + def num(tab: dict) -> int: + n = tab.get("number") + return n if isinstance(n, int) else 0 + + root = min(tabs, key=num) + active = next((t for t in tabs if t.get("tab_id") == active_tab_id), None) + if active is not None and active.get("tab_id") != root.get("tab_id"): + return active + return max(tabs, key=num) # root active (or unknown): newest tab (== root when alone) + + def _root_pane_id(result: dict) -> str | None: root = result.get("root_pane") if isinstance(root, dict) and isinstance(root.get("pane_id"), str): @@ -923,3 +1260,23 @@ def _drop_state(section: str, key: str) -> None: _save_state(state) except OSError: pass + + +def _drop_windows_for_workspace(workspace_id: str) -> None: + """Best-effort sidecar + return-file cleanup for every window of a closed + workspace (pane ids are ``:p`` — the same prefix rule the + poller registry uses). Never raises.""" + prefix = workspace_id + ":" + doomed: list[str] = [] + try: + with _state_lock(): + state = _load_state() + doomed = [key for key in state["windows"] if key.startswith(prefix)] + for key in doomed: + del state["windows"][key] + if doomed: + _save_state(state) + except OSError: + return # unknown keys: the trailers' own `rm -f` self-cleans the files + for key in doomed: + _remove_return_file(key) diff --git a/src/bmad_loop/adapters/tmux_base.py b/src/bmad_loop/adapters/tmux_base.py index 6ba2985b..38937818 100644 --- a/src/bmad_loop/adapters/tmux_base.py +++ b/src/bmad_loop/adapters/tmux_base.py @@ -379,7 +379,13 @@ def current_session(self) -> str | None: def _display_message(self, fmt: str) -> str | None: """Resolve a tmux format string against this process's client, or None - when not inside tmux / tmux is unavailable.""" + when not inside tmux / tmux is unavailable. The TMUX guard is what makes + "not inside" honest: against a live server, display-message would answer + for some OTHER client's session and misreport a plain shell as being + inside tmux — callers (in_ctl_session, the attach return-pane recording) + branch on exactly that distinction.""" + if not os.environ.get("TMUX"): + return None try: proc = self._run(["display-message", "-p", fmt], check=False) except (subprocess.SubprocessError, OSError): diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 15a800b9..6ca5270d 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -1346,11 +1346,13 @@ def cmd_attach(args: argparse.Namespace) -> int: # Record where to send the client once the sweep finishes this cycle's # decisions (see launch.return_attached_client), so answering them hands the # terminal back instead of stranding the user in the orchestrator window. + # Backend-honest inside-the-multiplexer probe: current_pane_id() is None + # outside, so a resolvable own pane means "switch the client back here", + # anything else means a throwaway client was attached and must detach. if return_window is not None: - if os.environ.get("TMUX"): - pane = launch.current_pane_id() - if pane is not None: - launch.set_return_pane(return_window, pane) + pane = launch.current_pane_id() + if pane is not None: + launch.set_return_pane(return_window, pane) else: launch.set_return_pane(return_window, launch.RETURN_DETACH) return subprocess.call(argv) diff --git a/src/bmad_loop/tui/app.py b/src/bmad_loop/tui/app.py index 7d5cefa5..43ce4f12 100644 --- a/src/bmad_loop/tui/app.py +++ b/src/bmad_loop/tui/app.py @@ -8,7 +8,6 @@ from __future__ import annotations -import os import shlex import subprocess import time @@ -379,14 +378,16 @@ def action_attach(self) -> None: def _attach_to_target(self, target: str, return_window: str | None = None) -> None: argv = runs.attach_target_argv(target) - if os.environ.get("TMUX"): - # switch-client is fire-and-forget; record our own pane on the ctl - # window so its trailing shell switches the client back here when it - # exits, instead of stranding the user in the control session. + # Backend-honest inside-the-multiplexer probe (current_pane_id() is None + # outside): inside, attach_target_argv returned the fire-and-forget + # switch/focus form, so no suspend is needed. + pane = launch.current_pane_id() + if pane is not None: + # Record our own pane on the ctl window so its trailing shell + # switches the client back here when it exits, instead of stranding + # the user in the control session. if return_window is not None: - pane = launch.current_pane_id() - if pane is not None: - launch.set_return_pane(return_window, pane) + launch.set_return_pane(return_window, pane) subprocess.call(argv) return # Outside tmux we attach a throwaway client (under suspend). The ctl diff --git a/src/bmad_loop/tui/launch.py b/src/bmad_loop/tui/launch.py index 024bf96c..a79af20f 100644 --- a/src/bmad_loop/tui/launch.py +++ b/src/bmad_loop/tui/launch.py @@ -12,7 +12,6 @@ from __future__ import annotations -import os import re import subprocess import sys @@ -99,8 +98,10 @@ def current_session() -> str | None: def in_ctl_session() -> bool: """True when we are running inside a control-session window (i.e. launched - detached by the TUI), as opposed to a user's own shell.""" - return bool(os.environ.get("TMUX")) and current_session() == CTL_SESSION + detached by the TUI), as opposed to a user's own shell. Backend-honest: + current_session() is None whenever this process is not inside the selected + multiplexer, so no direct TMUX/HERDR_* env sniffing happens here.""" + return current_session() == CTL_SESSION def detach_client() -> None: diff --git a/tests/test_herdr_backend.py b/tests/test_herdr_backend.py index ccb0a1f4..86410f3d 100644 --- a/tests/test_herdr_backend.py +++ b/tests/test_herdr_backend.py @@ -11,6 +11,7 @@ import json import os +import shlex import subprocess from pathlib import Path @@ -39,7 +40,8 @@ class FakeHerdr: def __init__(self, *, running: bool = True, protocol: int = 16) -> None: self.running = running self.protocol = protocol - self.workspaces: list[dict] = [] # {"label","workspace_id"} + self.workspaces: list[dict] = [] # {"label","workspace_id","active_tab_id"} + self.tabs: list[dict] = [] # {"tab_id","label","number","workspace_id","focused"} self.panes: list[dict] = [] # {"pane_id","workspace_id","tab_id","terminal_id"} self.calls: list[list[str]] = [] # Scripted `pane read` output per pane: a list of successive raw-text @@ -53,9 +55,30 @@ def __init__(self, *, running: bool = True, protocol: int = 16) -> None: # ---- helpers a test can call to seed state - def _new_tab_id(self, wid: str) -> str: + def _new_tab(self, wid: str, label: str | None, *, focus: bool) -> dict: self._tab_seq[wid] = self._tab_seq.get(wid, 0) + 1 - return f"{wid}:t{self._tab_seq[wid]}" + number = self._tab_seq[wid] + tab = { + # real herdr gives an unlabelled tab its NUMBER as the label (a + # workspace's root shell tab is "1" — verified 0.7.3) + "tab_id": f"{wid}:t{number}", + "label": label if label is not None else str(number), + "number": number, + "workspace_id": wid, + "focused": False, + } + self.tabs.append(tab) + if focus: + self._focus_tab(tab) + return tab + + def _focus_tab(self, tab: dict) -> None: + for other in self.tabs: + if other["workspace_id"] == tab["workspace_id"]: + other["focused"] = other is tab + for ws in self.workspaces: + if ws["workspace_id"] == tab["workspace_id"]: + ws["active_tab_id"] = tab["tab_id"] def _new_pane(self, wid: str, tab_id: str) -> dict: self._pane_seq[wid] = self._pane_seq.get(wid, 0) + 1 @@ -73,8 +96,9 @@ def add_workspace(self, label: str) -> str: """Create a workspace (with its root shell tab+pane) and return its id.""" self._ws_seq += 1 wid = f"w{self._ws_seq}" - self.workspaces.append({"label": label, "workspace_id": wid}) - self._new_pane(wid, self._new_tab_id(wid)) + self.workspaces.append({"label": label, "workspace_id": wid, "active_tab_id": None}) + tab = self._new_tab(wid, None, focus=True) # the root shell tab, label "1" + self._new_pane(wid, tab["tab_id"]) return wid def set_pane_reads(self, pane_id: str, screens: list[str]) -> None: @@ -141,14 +165,32 @@ def _dispatch(self, cmd, argv: list[str]) -> subprocess.CompletedProcess: if group == "workspace" and verb == "close": wid = argv[2] self.workspaces = [w for w in self.workspaces if w["workspace_id"] != wid] + self.tabs = [t for t in self.tabs if t["workspace_id"] != wid] self.panes = [p for p in self.panes if p["workspace_id"] != wid] return self._ok(cmd, {"type": "ok"}) if group == "tab" and verb == "create": wid = _flag(argv, "--workspace") - root = self._new_pane(wid, self._new_tab_id(wid)) - return self._ok(cmd, {"tab": {"tab_id": root["tab_id"]}, "root_pane": root}) + label = _flag(argv, "--label") if "--label" in argv else None + tab = self._new_tab(wid, label, focus="--focus" in argv) + root = self._new_pane(wid, tab["tab_id"]) + return self._ok(cmd, {"tab": tab, "root_pane": root}) + if group == "tab" and verb == "list": + # real herdr RAISES workspace_not_found for an absent --workspace + # (never []); the backend must therefore always enumerate bare. + if "--workspace" in argv: + wid = _flag(argv, "--workspace") + if all(w["workspace_id"] != wid for w in self.workspaces): + return self._server_err( + cmd, "workspace_not_found", f"workspace {wid} not found" + ) + return self._ok(cmd, {"tabs": [t for t in self.tabs if t["workspace_id"] == wid]}) + return self._ok(cmd, {"tabs": self.tabs}) if group == "tab" and verb == "focus": - return self._ok(cmd, {"type": "ok"}) + tab = next((t for t in self.tabs if t["tab_id"] == argv[2]), None) + if tab is None: + return self._server_err(cmd, "tab_not_found", f"tab {argv[2]} not found") + self._focus_tab(tab) + return self._ok(cmd, {"tab": tab, "type": "tab_info"}) if group == "pane" and verb == "list": return self._ok(cmd, {"panes": self.panes}) if group == "pane" and verb == "get": @@ -157,7 +199,22 @@ def _dispatch(self, cmd, argv: list[str]) -> subprocess.CompletedProcess: return self._server_err(cmd, "pane_not_found", f"pane {argv[2]} not found") return self._ok(cmd, {"pane": pane}) if group == "pane" and verb == "close": + pane = next((p for p in self.panes if p["pane_id"] == argv[2]), None) self.panes = [p for p in self.panes if p["pane_id"] != argv[2]] + if pane is not None: # a pane close cascades its now-empty tab closed + tab_id = pane["tab_id"] + if all(p["tab_id"] != tab_id for p in self.panes): + self.tabs = [t for t in self.tabs if t["tab_id"] != tab_id] + for ws in self.workspaces: + if ws.get("active_tab_id") == tab_id: + ws["active_tab_id"] = next( + ( + t["tab_id"] + for t in self.tabs + if t["workspace_id"] == ws["workspace_id"] + ), + None, + ) return self._ok(cmd, {"type": "ok"}) if group == "pane" and verb == "read": pane_id = argv[2] @@ -360,7 +417,7 @@ def test_seam_methods_never_leak_raw_subprocess_error(boom, tmp_path): lambda: mux.new_session("s", tmp_path), lambda: mux.new_window("s", "n", tmp_path, {}, "cmd"), lambda: mux.send_text("w1:p1", "hi"), - lambda: mux.new_parked_window("s", "n", tmp_path, ["echo", "hi"], ""), # always raises + lambda: mux.new_parked_window("s", "n", tmp_path, ["echo", "hi"], ""), ] for call in raisers: with pytest.raises(MultiplexerError) as excinfo: @@ -586,30 +643,179 @@ def test_protocol_above_supported_warns_but_proceeds(fake): # --------------------------------------------------------------- degradations -def test_new_parked_window_raises(fake, tmp_path): - with pytest.raises(HerdrError): - HerdrMultiplexer().new_parked_window("s", "n", tmp_path, ["echo", "hi"], "") - - -def test_pipe_pane_tolerates_dead_pane_and_detach_switch_noop(fake, tmp_path): +def test_pipe_pane_tolerates_dead_pane_and_detach_noop(fake, tmp_path): # pipe_pane races a pane that already died on launch: the priming read gets # pane_not_found, so no tee thread is spun up (tmux swallows the same race). mux = HerdrMultiplexer() assert mux.pipe_pane("w9:p9", tmp_path / "log") is None assert mux._pollers == {} # nothing left running assert mux.detach_client() is None - assert mux.switch_client("w1:p1") is False -def test_attach_target_argv_resolves_terminal(fake): - fake.add_workspace("bmad-loop-x") - pane = fake.panes[-1] +# ------------------------------------------------- TUI-launch surface (PR 2) + + +def test_new_parked_window_recipe(fake): + fake.add_workspace("bmad-loop-ctl") mux = HerdrMultiplexer() - assert mux.attach_target_argv(pane["pane_id"]) == [ - "herdr", "terminal", "attach", pane["terminal_id"], + pane_id = mux.new_parked_window( + "bmad-loop-ctl", "run-RID", Path("/work"), ["python", "-m", "x"], RETURN_OPTION + ) + (tab_create,) = _creates(fake, "tab", "create") + assert tab_create == [ + "tab", "create", + "--workspace", "w1", + "--label", "run-RID", + "--cwd", str(Path("/work")), + "--no-focus", ] # fmt: skip - # a tmux-style '=' prefix a caller might pass is stripped - assert mux.attach_target_argv("=" + pane["pane_id"])[-1] == pane["terminal_id"] + (pane_run,) = _creates(fake, "pane", "run") + assert pane_run[:3] == ["pane", "run", pane_id] + typed = pane_run[3] + # the typed line execs `sh -c ''` (one line, POSIX+fish-safe quoting) + assert typed.startswith("exec sh -c ") + sh_argv = shlex.split(typed[len("exec ") :]) + assert sh_argv[:2] == ["sh", "-c"] + src = sh_argv[2] + assert "\n" not in typed + # recipe shape: argv; exit capture; banner; park; return trailer + assert src.startswith("python -m x; ec=$?; ") + assert "[bmad-loop exited $ec — press enter]" in src + assert "read -r" in src + retfile = herdr_backend._return_file(pane_id) + assert str(retfile) in src # the trailer cats/rms the per-window return file + assert 'herdr tab focus "$ret"' in src + # the sidecar remembers which option this window's trailer consumes + state = json.loads(Path(os.environ["BMAD_LOOP_HERDR_STATE"]).read_text()) + assert state["windows"][pane_id][herdr_backend._PARKED_RETURN_KEY] == RETURN_OPTION + + +def test_new_parked_window_missing_session_raises(fake, tmp_path): + with pytest.raises(HerdrError): + HerdrMultiplexer().new_parked_window("s", "n", tmp_path, ["echo", "hi"], RETURN_OPTION) + + +def test_parked_return_option_round_trip(fake): + # cmd_attach / the TUI write the return option by tmux-style name target; + # return_attached_client (inside the window) reads it back by native id — + # the sidecar key is normalized so both agree. The return FILE holds the + # trailer-actionable TAB id of the origin pane, resolved at write time. + fake.add_workspace("bmad-loop-ctl") + fake.add_workspace("userws") + origin = fake.panes[-1] # the TUI's own pane, in the user's workspace + mux = HerdrMultiplexer() + pane_id = mux.new_parked_window("bmad-loop-ctl", "sweep-RID", Path("/w"), ["x"], RETURN_OPTION) + retfile = herdr_backend._return_file(pane_id) + + mux.set_window_option("=bmad-loop-ctl:sweep-RID", RETURN_OPTION, origin["pane_id"]) + assert mux.show_window_option(pane_id, RETURN_OPTION) == origin["pane_id"] + assert retfile.read_text(encoding="utf-8").strip() == origin["tab_id"] + + # RETURN_DETACH is mirrored verbatim (the trailer treats it as "do nothing": + # ending the source closes the pane, which ends a `terminal attach` client) + mux.set_window_option(pane_id, RETURN_OPTION, "detach") + assert retfile.read_text(encoding="utf-8").strip() == "detach" + + # unsetting clears the option AND the file (so the post-exit trailer cannot + # fire a second time after a mid-process return) + mux.unset_window_option(pane_id, RETURN_OPTION) + assert mux.show_window_option(pane_id, RETURN_OPTION) == "" + assert not retfile.exists() + + +def test_parked_return_unresolvable_origin_clears_file(fake): + fake.add_workspace("bmad-loop-ctl") + mux = HerdrMultiplexer() + pane_id = mux.new_parked_window("bmad-loop-ctl", "run-RID", Path("/w"), ["x"], RETURN_OPTION) + mux.set_window_option(pane_id, RETURN_OPTION, "detach") + retfile = herdr_backend._return_file(pane_id) + assert retfile.exists() + # a gone origin pane must not leave a stale focus target behind + mux.set_window_option(pane_id, RETURN_OPTION, "w9:p9") + assert not retfile.exists() + assert mux.show_window_option(pane_id, RETURN_OPTION) == "w9:p9" # option itself kept + + +def test_kill_window_by_name_target_cleans_return_file(fake): + fake.add_workspace("bmad-loop-ctl") + mux = HerdrMultiplexer() + pane_id = mux.new_parked_window("bmad-loop-ctl", "run-RID", Path("/w"), ["x"], RETURN_OPTION) + mux.set_window_option(pane_id, RETURN_OPTION, "detach") + retfile = herdr_backend._return_file(pane_id) + assert retfile.exists() + mux.kill_window("=bmad-loop-ctl:run-RID") # kill_ctl_window passes this form + assert all(p["pane_id"] != pane_id for p in fake.panes) + assert not retfile.exists() + assert mux.show_window_option(pane_id, RETURN_OPTION) == "" + + +def test_kill_session_cleans_window_sidecar_and_return_files(fake): + fake.add_workspace("bmad-loop-ctl") + mux = HerdrMultiplexer() + pane_id = mux.new_parked_window("bmad-loop-ctl", "run-RID", Path("/w"), ["x"], RETURN_OPTION) + mux.set_window_option(pane_id, RETURN_OPTION, "detach") + retfile = herdr_backend._return_file(pane_id) + mux.kill_session("bmad-loop-ctl") + assert not retfile.exists() + state = json.loads(Path(os.environ["BMAD_LOOP_HERDR_STATE"]).read_text()) + assert pane_id not in state["windows"] + + +def test_select_window_by_name_focuses_tab(fake): + fake.add_workspace("bmad-loop-ctl") + mux = HerdrMultiplexer() + pane_id = mux.new_window("bmad-loop-ctl", "run-RID", Path("/w"), {}, "echo hi") + tab_id = next(p["tab_id"] for p in fake.panes if p["pane_id"] == pane_id) + mux.select_window("=bmad-loop-ctl:run-RID") + assert ["tab", "focus", tab_id] in fake.calls + ws = next(w for w in fake.workspaces if w["label"] == "bmad-loop-ctl") + assert ws["active_tab_id"] == tab_id + # unresolvable targets stay quiet no-ops (sentinel) + assert mux.select_window("=bmad-loop-ctl:nope") is None + assert mux.select_window("=absent:x") is None + + +def test_attach_target_argv_native_and_name_targets(fake): + fake.add_workspace("bmad-loop-ctl") + mux = HerdrMultiplexer() + pane_id = mux.new_window("bmad-loop-ctl", "run-RID", Path("/w"), {}, "echo hi") + pane = next(p for p in fake.panes if p["pane_id"] == pane_id) + expected = ["herdr", "terminal", "attach", pane["terminal_id"]] + assert mux.attach_target_argv(pane_id) == expected # native pane id + assert mux.attach_target_argv("=bmad-loop-ctl:run-RID") == expected # by window name + + +def test_attach_target_argv_session_level(fake): + fake.add_workspace("bmad-loop-ctl") + mux = HerdrMultiplexer() + a = mux.new_window("bmad-loop-ctl", "run-a", Path("/w"), {}, "sleep 1") + b = mux.new_window("bmad-loop-ctl", "run-b", Path("/w"), {}, "sleep 1") + term = {p["pane_id"]: p["terminal_id"] for p in fake.panes} + # task tabs are created --no-focus, so the root shell tab is still active: + # a session-level attach prefers the NEWEST task tab over the root shell + assert mux.attach_target_argv("=bmad-loop-ctl")[-1] == term[b] + # after an explicit select (attach_plan runs select_ctl_window first) the + # active tab wins + mux.select_window("=bmad-loop-ctl:run-a") + assert mux.attach_target_argv("=bmad-loop-ctl")[-1] == term[a] + assert a != b + + +def test_attach_target_argv_session_level_single_tab_is_root(fake): + fake.add_workspace("bmad-loop-x") + root = fake.panes[-1] + assert HerdrMultiplexer().attach_target_argv("=bmad-loop-x")[-1] == root["terminal_id"] + + +def test_attach_target_argv_inside_herdr_focuses(fake, monkeypatch): + # inside a herdr pane, nesting a full attach is the wrong move (as with + # attach-inside-tmux): return the fire-and-forget focus argv instead + fake.add_workspace("bmad-loop-ctl") + mux = HerdrMultiplexer() + pane_id = mux.new_window("bmad-loop-ctl", "run-RID", Path("/w"), {}, "echo hi") + tab_id = next(p["tab_id"] for p in fake.panes if p["pane_id"] == pane_id) + monkeypatch.setenv("HERDR_ENV", "1") + assert mux.attach_target_argv("=bmad-loop-ctl:run-RID") == ["herdr", "tab", "focus", tab_id] def test_attach_target_argv_missing_pane_raises(fake): @@ -617,6 +823,61 @@ def test_attach_target_argv_missing_pane_raises(fake): HerdrMultiplexer().attach_target_argv("w9:p9") +def test_attach_target_argv_missing_session_raises_with_guidance(fake): + with pytest.raises(HerdrError) as excinfo: + HerdrMultiplexer().attach_target_argv("=absent") + assert "attach" in str(excinfo.value) + + +def test_switch_client_focuses_origin_tab(fake): + fake.add_workspace("bmad-loop-ctl") + fake.add_workspace("userws") + origin = fake.panes[-1] + mux = HerdrMultiplexer() + assert mux.switch_client(origin["pane_id"]) is True + assert ["tab", "focus", origin["tab_id"]] in fake.calls + # a gone target is honestly False; herdr has no "last client" to fall back to + assert mux.switch_client("w9:p9") is False + assert mux.switch_client("w9:p9", last_fallback=True) is False + + +def test_list_windows_tmux_field_mapping(fake): + fake.add_workspace("bmad-loop-ctl") + mux = HerdrMultiplexer() + pane_id = mux.new_window("bmad-loop-ctl", "run-RID", Path("/w"), {}, "echo hi") + mux.set_window_option(pane_id, PROJECT_OPTION, "/proj") + rows = mux.list_windows("bmad-loop-ctl", ["window_id", "window_name", PROJECT_OPTION]) + by_id = {row[0]: row for row in rows} + assert by_id[pane_id] == (pane_id, "run-RID", "/proj") + root = fake.panes[0]["pane_id"] + assert by_id[root] == (root, "1", "") # the root shell tab: numeric label, untagged + # ctl_window()'s discovery contract: every window comes back with its name + assert {row[1] for row in rows} == {"1", "run-RID"} + + +def test_in_ctl_session_seam_honest_under_herdr(fake, monkeypatch): + # launch.in_ctl_session no longer sniffs TMUX: with herdr selected and the + # herdr pane env present it resolves through the seam. + from bmad_loop.adapters.multiplexer import get_multiplexer + from bmad_loop.tui import launch + + wid = fake.add_workspace(launch.CTL_SESSION) + monkeypatch.delenv("TMUX", raising=False) + monkeypatch.setenv("BMAD_LOOP_MUX_BACKEND", "herdr") + get_multiplexer.cache_clear() + try: + monkeypatch.setenv("HERDR_ENV", "1") + monkeypatch.setenv("HERDR_WORKSPACE_ID", wid) + monkeypatch.setenv("HERDR_PANE_ID", "w1:p1") + assert launch.in_ctl_session() is True + monkeypatch.setenv("HERDR_WORKSPACE_ID", "w9") # some other workspace + assert launch.in_ctl_session() is False + monkeypatch.delenv("HERDR_ENV", raising=False) # not inside herdr at all + assert launch.in_ctl_session() is False + finally: + get_multiplexer.cache_clear() + + def test_current_accessors_resolve_from_env(fake, monkeypatch): wid = fake.add_workspace("bmad-loop-x") monkeypatch.setenv("HERDR_ENV", "1") diff --git a/tests/test_herdr_integration.py b/tests/test_herdr_integration.py index b5570419..9407550b 100644 --- a/tests/test_herdr_integration.py +++ b/tests/test_herdr_integration.py @@ -219,3 +219,64 @@ def test_herdr_pipe_pane_log_grows_under_real_pane(tmp_path, herdr_session, monk # and spin subprocess reads for the rest of the pytest worker's life. mux.kill_session(session) assert mux._pollers == {} + + +def test_herdr_parked_window_via_start_detached(herdr_session, tmp_path): + """PR-2 surface, live: launch.start_detached parks `bmad-loop validate` in a + bmad-loop-ctl workspace tab. Pins the whole chain: tab-label discovery + (ctl_window through the list_windows join), the exit banner rendering while + the window PARKS (stays alive) after the command exits, set_return_pane by + tmux-style name target mirroring into the per-window return file, and Enter + unparking — the trailer consumes the file, the pane closes (which is what + ends a watching `terminal attach` client), and the ctl workspace survives + on its root shell tab.""" + import re + + from bmad_loop.tui import launch + + run_id = "20260713-000000-p6live" + win_id = launch.start_detached( + tmp_path, ["validate", "--project", str(tmp_path)], run_id, "run" + ) + assert win_id # the native window id (the tab's root pane) + mux = multiplexer.get_multiplexer() + assert isinstance(mux, herdr_backend.HerdrMultiplexer) # no silent tmux fall-back + + # discovery: the ctl window comes back by its tab label + assert launch.ctl_window(run_id) == f"run-{run_id}" + + # validate exits quickly; the window must PARK with the EXPANDED banner + # (the typed recipe itself contains the literal `$ec` text, so match the + # substituted exit status, not the echo's source) + banner_re = re.compile(r"\[bmad-loop exited \d+") + deadline = time.monotonic() + 30 + screen = "" + while time.monotonic() < deadline: + screen = subprocess.run( + ["herdr", "pane", "read", win_id, "--source", "recent-unwrapped"], + capture_output=True, + text=True, + ).stdout + if banner_re.search(screen): + break + time.sleep(0.5) + assert banner_re.search(screen), f"no parked banner in pane after validate: {screen[-500:]!r}" + assert mux.window_alive(launch.CTL_SESSION, win_id) is True # parked, not closed + + # attach-time return recording by tmux-style name target lands in the + # per-window return file (the sidecar key is normalized to the native id) + launch.set_return_pane(f"={launch.CTL_SESSION}:run-{run_id}", launch.RETURN_DETACH) + retfile = herdr_backend._return_file(win_id) + assert retfile.read_text(encoding="utf-8").strip() == launch.RETURN_DETACH + assert mux.show_window_option(win_id, launch.RETURN_OPTION) == launch.RETURN_DETACH + + # Enter unparks: the trailer consumes the file, sh exits, the pane closes + subprocess.run(["herdr", "pane", "send-keys", win_id, "enter"], capture_output=True) + deadline = time.monotonic() + 15 + while time.monotonic() < deadline: + if not mux.window_alive(launch.CTL_SESSION, win_id) and not retfile.exists(): + break + time.sleep(0.5) + assert mux.window_alive(launch.CTL_SESSION, win_id) is False + assert not retfile.exists() # the trailer rm -f'd it + assert mux.has_session(launch.CTL_SESSION) is True # root shell tab keeps it alive diff --git a/tests/test_tui_launch.py b/tests/test_tui_launch.py index fd0df45b..4e862cc8 100644 --- a/tests/test_tui_launch.py +++ b/tests/test_tui_launch.py @@ -248,14 +248,29 @@ def test_current_pane_id_reads_pane(monkeypatch): def fake(argv, **kwargs): return subprocess.CompletedProcess(argv, 0, stdout="%9\n", stderr="") + monkeypatch.setenv("TMUX", "/tmp/tmux-1000/default,123,0") # inside tmux monkeypatch.setattr(tmux_base.subprocess, "run", fake) assert launch.current_pane_id() == "%9" def test_current_pane_id_none_outside_tmux(monkeypatch): + # Outside tmux the TMUX guard answers None WITHOUT shelling out: against a + # live server, display-message would answer for some OTHER client's session + # and misreport a plain shell as being inside tmux. + def boom(*_a, **_k): + raise AssertionError("outside tmux, current_* must not shell out") + + monkeypatch.delenv("TMUX", raising=False) + monkeypatch.setattr(tmux_base.subprocess, "run", boom) + assert launch.current_pane_id() is None + assert launch.current_session() is None + + +def test_current_pane_id_none_on_transport_failure(monkeypatch): def fake(argv, **kwargs): return subprocess.CompletedProcess(argv, 1, stdout="", stderr="no server") + monkeypatch.setenv("TMUX", "/tmp/tmux-1000/default,123,0") monkeypatch.setattr(tmux_base.subprocess, "run", fake) assert launch.current_pane_id() is None @@ -296,6 +311,7 @@ def fake(argv, **kwargs): killed.append(list(argv)) return subprocess.CompletedProcess(argv, 0, stdout="", stderr="") + monkeypatch.setenv("TMUX", "/tmp/tmux-1000/default,123,0") # we sit in a pane of @4 monkeypatch.setattr(tmux_base.subprocess, "run", fake) monkeypatch.setattr(tmux_base.shutil, "which", lambda name: f"/usr/bin/{name}") @@ -342,6 +358,7 @@ def fake(argv, **kwargs): killed.append(list(argv)) return subprocess.CompletedProcess(argv, 0, stdout="", stderr="") + monkeypatch.setenv("TMUX", "/tmp/tmux-1000/default,123,0") monkeypatch.setattr(tmux_base.subprocess, "run", fake) monkeypatch.setattr(tmux_base.shutil, "which", lambda name: f"/usr/bin/{name}") @@ -365,14 +382,28 @@ def test_select_ctl_window_id_argv(fake_run): def test_in_ctl_session(monkeypatch): + # in_ctl_session is backend-honest: it trusts current_session(), which is + # None whenever this process is not inside the selected multiplexer (the + # old direct TMUX sniff lives in the tmux backend's _display_message now — + # see test_in_ctl_session_outside_tmux). monkeypatch.setattr(launch, "current_session", lambda: "bmad-loop-ctl") - monkeypatch.setenv("TMUX", "/tmp/tmux-1000/default,123,0") assert launch.in_ctl_session() is True monkeypatch.setattr(launch, "current_session", lambda: "some-other-session") assert launch.in_ctl_session() is False + monkeypatch.setattr(launch, "current_session", lambda: None) + assert launch.in_ctl_session() is False # not inside the multiplexer + + +def test_in_ctl_session_outside_tmux(monkeypatch): + # End-to-end through the real tmux backend: outside tmux (no TMUX env) the + # backend's current_session() is None without shelling out, even when a + # live server would answer display-message for some other client. + def boom(*_a, **_k): + raise AssertionError("outside tmux, in_ctl_session must not shell out") + monkeypatch.delenv("TMUX", raising=False) - monkeypatch.setattr(launch, "current_session", lambda: "bmad-loop-ctl") - assert launch.in_ctl_session() is False # not inside tmux + monkeypatch.setattr(tmux_base.subprocess, "run", boom) + assert launch.in_ctl_session() is False def test_detach_client_argv(fake_run): @@ -382,7 +413,10 @@ def test_detach_client_argv(fake_run): def _return_fake(monkeypatch, *, win="@5", option="%9", switch_rc=0): """Script tmux for return_attached_client: display-message -> window id, - show-options -> the recorded RETURN_OPTION, switch-client -> switch_rc.""" + show-options -> the recorded RETURN_OPTION, switch-client -> switch_rc. + return_attached_client runs inside a ctl window, so TMUX is set (the + backend's current_window_id answers None otherwise).""" + monkeypatch.setenv("TMUX", "/tmp/tmux-1000/default,123,0") calls: list[list[str]] = [] def fake(argv, **kwargs): From 8ed66e8945b4c1fb28a6dccedd5516daf4998b57 Mon Sep 17 00:00:00 2001 From: pbean Date: Tue, 14 Jul 2026 09:57:07 -0700 Subject: [PATCH 2/2] fix(mux): parked-window rollback + TUI attach guard (review round 2) CodeRabbit round-2 fixes on the TUI-launch surface: - new_parked_window: the herdr launch is three steps (tab create -> sidecar write -> typed recipe) where tmux gets one atomic new-window; a failure after tab create now rolls the fresh tab back via the existing kill_window teardown (close + sidecar prune + return-file cleanup) instead of leaving an untracked idle shell in the ctl workspace until workspace close. - TUI attach: attach_target_argv is a server round-trip on herdr and can raise MultiplexerError after the availability/session pre-gates pass; _attach_to_target now surfaces it as an error toast instead of crashing the app. The cmd_attach half of the finding was declined: cli.main's backstop already prints 'error: ' + rc 1 (a new regression test pins that). - ROADMAP: the herdr status paragraph now matches the header line (shipped end-to-end on POSIX: run path + TUI-launch surface). Full suite 2310 green with and without TMUX in the env; trunk clean. --- docs/ROADMAP.md | 3 +- src/bmad_loop/adapters/herdr_backend.py | 30 ++++++++++------ src/bmad_loop/tui/app.py | 7 +++- tests/test_cli.py | 16 +++++++++ tests/test_herdr_backend.py | 48 +++++++++++++++++++++++++ tests/test_tui_app.py | 26 ++++++++++++++ 6 files changed, 118 insertions(+), 12 deletions(-) diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 037f5ebc..f2a42a0e 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -43,7 +43,8 @@ availability-aware with discriminating `available()` probes (psmux → `which("psmux") and which("tmux") and which("pwsh")`; tmux-windows → `which("tmux") and not which("psmux")`) and an explicit `bmad-loop mux set ` tie-break. -The third — **herdr** — has **shipped** (for the engine run path) as a +The third — **herdr** — has **shipped** end-to-end on POSIX (engine run path + +TUI-launch surface) as a different-binary-family backend: it implements `TerminalMultiplexer` fresh over herdr's workspace/tab/pane model rather than subclassing `BaseTmuxBackend`, and probes `which("herdr")`, so it is pairwise-discriminating against the tmux family by construction diff --git a/src/bmad_loop/adapters/herdr_backend.py b/src/bmad_loop/adapters/herdr_backend.py index 49bfa5fe..768d824e 100644 --- a/src/bmad_loop/adapters/herdr_backend.py +++ b/src/bmad_loop/adapters/herdr_backend.py @@ -848,17 +848,27 @@ def new_parked_window( raise HerdrError(f"herdr tab create did not return a root pane id: {result!r}") # Record which option this window's trailer consumes BEFORE typing the # recipe, so a set_window_option racing the launch already mirrors into - # the return file. + # the return file. tmux gets create+launch atomically (one new-window + # call); here the tab already exists, so any failure before the recipe + # is typed must roll it back or it lingers as an untracked idle shell + # until the whole workspace is closed. try: - with _state_lock(): - state = _load_state() - state["windows"].setdefault(pane_id, {})[_PARKED_RETURN_KEY] = return_opt - _save_state(state) - except OSError as exc: - raise HerdrError(f"herdr sidecar write failed: {exc}") from exc - self._client._herdr( - "pane", "run", pane_id, "exec sh -c " + shlex.quote(_parked_source(argv, pane_id)) - ) + try: + with _state_lock(): + state = _load_state() + state["windows"].setdefault(pane_id, {})[_PARKED_RETURN_KEY] = return_opt + _save_state(state) + except OSError as exc: + raise HerdrError(f"herdr sidecar write failed: {exc}") from exc + self._client._herdr( + "pane", "run", pane_id, "exec sh -c " + shlex.quote(_parked_source(argv, pane_id)) + ) + except MultiplexerError: + try: + self.kill_window(pane_id) + except MultiplexerError: + pass + raise return pane_id def list_window_ids(self, session: str) -> list[str]: diff --git a/src/bmad_loop/tui/app.py b/src/bmad_loop/tui/app.py index 43ce4f12..daab212a 100644 --- a/src/bmad_loop/tui/app.py +++ b/src/bmad_loop/tui/app.py @@ -21,6 +21,7 @@ from tomlkit.exceptions import ParseError from .. import bmadconfig, decisions, devcontract, policy, resolve, runs, stories, verify +from ..adapters.multiplexer import MultiplexerError from ..journal import load_state from ..model import ( PAUSE_EPIC_BOUNDARY, @@ -377,7 +378,11 @@ def action_attach(self) -> None: self._attach_to_target(target) def _attach_to_target(self, target: str, return_window: str | None = None) -> None: - argv = runs.attach_target_argv(target) + try: + argv = runs.attach_target_argv(target) + except MultiplexerError as e: + self.notify(str(e), severity="error") + return # Backend-honest inside-the-multiplexer probe (current_pane_id() is None # outside): inside, attach_target_argv returned the fire-and-forget # switch/focus form, so no suspend is needed. diff --git a/tests/test_cli.py b/tests/test_cli.py index 6bb271e8..2b5301fe 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -498,6 +498,22 @@ def test_attach_nothing_to_attach(project, monkeypatch, capsys): assert "nothing to attach" in capsys.readouterr().err +def test_attach_multiplexer_error_surfaces_clean_error(project, monkeypatch, capsys): + # attach_plan reaches the multiplexer (a herdr server round-trip); when that + # raises, main()'s backstop must surface `error: ` + rc 1, never a + # traceback to the parked control pane. + from bmad_loop.adapters.multiplexer import MultiplexerError + from bmad_loop.tui import launch + + def boom(_proj, _rid): + raise MultiplexerError("herdr server not reachable") + + _make_run_with_decision(project, run_id="20260101-000000-aaaa") + monkeypatch.setattr(launch, "attach_plan", boom) + assert cli.main(["attach", "--project", str(project.project), "20260101-000000-aaaa"]) == 1 + assert "error: herdr server not reachable" in capsys.readouterr().err + + def test_sweep_dry_run_lists_open_entries(project, capsys): from conftest import write_ledger diff --git a/tests/test_herdr_backend.py b/tests/test_herdr_backend.py index 86410f3d..e91370f9 100644 --- a/tests/test_herdr_backend.py +++ b/tests/test_herdr_backend.py @@ -695,6 +695,54 @@ def test_new_parked_window_missing_session_raises(fake, tmp_path): HerdrMultiplexer().new_parked_window("s", "n", tmp_path, ["echo", "hi"], RETURN_OPTION) +def test_new_parked_window_rolls_back_tab_when_recipe_typing_fails(fake, monkeypatch): + # tmux gets create+launch atomically (one new-window call); here the tab + # already exists when the recipe-typing `pane run` fails, so the backend + # must close it again — not leave an untracked idle shell in the ctl + # workspace with a stale sidecar entry. + fake.add_workspace("bmad-loop-ctl") + before = {p["pane_id"] for p in fake.panes} + real_dispatch = fake._dispatch + seen: dict[str, str] = {} + + def failing_pane_run(cmd, argv): + if argv[:2] == ["pane", "run"]: + seen["pane_id"] = argv[2] + return fake._server_err(cmd, "internal", "pane run exploded") + return real_dispatch(cmd, argv) + + monkeypatch.setattr(fake, "_dispatch", failing_pane_run) + with pytest.raises(HerdrError): + HerdrMultiplexer().new_parked_window( + "bmad-loop-ctl", "run-RID", Path("/w"), ["x"], RETURN_OPTION + ) + assert seen["pane_id"] not in before # the launch really created a fresh pane + assert {p["pane_id"] for p in fake.panes} == before # ...and rolled it back + assert _creates(fake, "pane", "close") # via an explicit close, not luck + state = json.loads(Path(os.environ["BMAD_LOOP_HERDR_STATE"]).read_text()) + assert seen["pane_id"] not in state["windows"] # sidecar entry pruned too + + +def test_new_parked_window_rolls_back_tab_when_sidecar_write_fails(fake, monkeypatch): + # The sidecar write sits between tab create and recipe typing; when it + # fails there is no sidecar entry to catch the tab later, so the rollback + # is the only thing standing between us and an orphan. + fake.add_workspace("bmad-loop-ctl") + before = {p["pane_id"] for p in fake.panes} + + def no_lock(_path, **_kw): + raise OSError("lock unavailable") + + monkeypatch.setattr(herdr_backend.platform_util, "file_lock", no_lock) + with pytest.raises(HerdrError): + HerdrMultiplexer().new_parked_window( + "bmad-loop-ctl", "run-RID", Path("/w"), ["x"], RETURN_OPTION + ) + assert {p["pane_id"] for p in fake.panes} == before # created tab rolled back + assert _creates(fake, "pane", "close") + assert not _creates(fake, "pane", "run") # recipe never typed into a doomed tab + + def test_parked_return_option_round_trip(fake): # cmd_attach / the TUI write the return option by tmux-style name target; # return_attached_client (inside the window) reads it back by native id — diff --git a/tests/test_tui_app.py b/tests/test_tui_app.py index 53522074..1311cbfd 100644 --- a/tests/test_tui_app.py +++ b/tests/test_tui_app.py @@ -31,6 +31,7 @@ ) from bmad_loop import policy as policy_mod +from bmad_loop.adapters.multiplexer import MultiplexerError from bmad_loop.journal import Journal, save_state from bmad_loop.model import Phase, RunState, StoryTask, TokenUsage from bmad_loop.runs import RUNS_DIR @@ -1217,6 +1218,31 @@ async def test_attach_without_agent_session_notifies(project, monkeypatch): await until(pilot, lambda: any("no live agent session" in m for m in notifications(app))) +async def test_attach_multiplexer_error_notifies(project, monkeypatch): + # attach_target_argv is a server round-trip on the herdr backend, so it can + # raise after the availability/session pre-gates pass (server died or the + # workspace was torn down in between); the TUI must surface the error as a + # toast, not crash the app. + monkeypatch.setattr(launch, "tmux_available", lambda: True) + monkeypatch.setattr(launch, "session_exists", lambda session: True) + monkeypatch.setattr(launch, "ctl_window", lambda run_id: None) + + def boom(_target): + raise MultiplexerError("herdr server not reachable") + + monkeypatch.setattr("bmad_loop.tui.app.runs.attach_target_argv", boom) + make_run(project.project, "20260611-100000-aaaa") + app = BmadLoopApp(project.project) + async with app.run_test() as pilot: + await until(pilot, lambda: isinstance(app.screen, DashboardScreen)) + await until(pilot, lambda: dashboard(app).selected_run_id is not None) + await pilot.press("a") + await until( + pilot, lambda: any("herdr server not reachable" in m for m in notifications(app)) + ) + assert isinstance(app.screen, DashboardScreen) # the action failed soft + + # ------------------------------------------------------- sweep decision flow