Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,15 @@ 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.

- **Seam-canonical window targets.** The `=session[:window]` target grammar is now owned by the
`TerminalMultiplexer` seam instead of living as hand-assembled tmux syntax in core: a new
concrete `target(session, window=None)` encoder (overridable per backend, tmux inherits the
default and passes it straight through) and a module-level `parse_target()` decoder that
native-id backends reuse instead of re-deriving the grammar (the herdr backend's
`_parse_target` now delegates to it). `runs.py`/`tui/launch.py`/`tui/app.py` format every
target via `target()` (new `runs.session_target` / `launch.ctl_target` helpers) — output is
byte-identical, so no backend or operator behavior changes; the contract is documented in the
adapter authoring guide's new "Window targets" section.
- **Herdr TUI-launch surface.** The herdr backend now covers everything `tui/launch.py` drives:
parked orchestrator windows (a typed `exec sh -c '<argv>; banner; read; trailer'` recipe,
tmux-identical from the operator's seat, with the return-to-origin target mirrored into a
Expand Down
16 changes: 16 additions & 0 deletions docs/adapter-authoring-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,22 @@ seams of a full OS port are in
`detach_client`, `switch_client` (with an optional last-client fallback),
`available` (is this backend usable on the current host).

**Window targets.** The target-taking methods (`kill_window`, `select_window`,
the window-option trio, `attach_target_argv`, `switch_client`) receive one of two
families: the **seam-canonical target token** `=session[:window]` — formatted by
the concrete `TerminalMultiplexer.target(session, window=None)`, decoded by the
module-level `parse_target()` — or the backend's own **native id** (whatever your
`new_window` returned). Core never hand-assembles the grammar; it calls
`target()`. tmux consumes the token natively (it coincides with tmux exact-match
syntax), so `BaseTmuxBackend` passes it straight through. A native-id backend
calls `parse_target()` first — `None` means "already a native id, use as-is",
otherwise resolve `(session, window)` yourself; `herdr_backend._parse_target` is
the worked example (workspace-by-label → tab-by-name → root pane, resolved lazily
at use time). You MAY override `target()` to emit native ids, but the token must
stay a stable _by-name_ reference: core formats targets ahead of use (a parked
window's return target, for one), so eager resolution to a live id goes stale —
inheriting the default and resolving lazily is almost always right.

Operations that can race a window dying (`pipe_pane`) or a session already being
gone (`kill_session`) must tolerate it rather than raise; everything else raises a
`MultiplexerError` subclass on failure, which call sites catch at the seam (e.g.
Expand Down
32 changes: 17 additions & 15 deletions src/bmad_loop/adapters/herdr_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@
``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",
seam-canonical ``=session[:window]`` tokens core formats via
``TerminalMultiplexer.target`` — see ``_parse_target``): outside herdr it
returns ``["herdr", "terminal",
"attach", <terminal_id>]`` (which blocks, and exits when the pane closes);
inside a herdr pane it returns the fire-and-forget ``["herdr", "tab",
"focus", <tab_id>]`` — the switch-client move, mirroring tmux's in-``TMUX``
Expand Down Expand Up @@ -88,7 +89,7 @@
from pathlib import Path

from .. import platform_util
from .multiplexer import MultiplexerError, TerminalMultiplexer
from .multiplexer import MultiplexerError, TerminalMultiplexer, parse_target

HERDR_TIMEOUT_S = 30
# The herdr server wire protocol this backend was written against (0.7.3). Read
Expand Down Expand Up @@ -665,19 +666,20 @@ def _tab_root_pane(self, tab_id: str, *, strict: bool) -> str | 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("="):
Callers hand this two families: the seam-canonical ``=session[:window]``
token (decoded by :func:`multiplexer.parse_target`; core formats these
via ``TerminalMultiplexer.target``) and our own native pane id — 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)."""
parsed = parse_target(target)
if parsed is None:
return target
session, _, window = target[1:].partition(":")
session, window = parsed
row = self._workspace_row(session, strict=strict)
if row is None:
if strict:
Expand Down
55 changes: 48 additions & 7 deletions src/bmad_loop/adapters/multiplexer.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,24 @@ class MultiplexerError(Exception):
without importing a backend."""


def parse_target(target: str) -> tuple[str, str | None] | None:
"""Decode a seam-canonical window target (see :meth:`TerminalMultiplexer.target`).

Returns ``(session, window)`` for a canonical ``=session[:window]`` token —
``window`` is None when absent *or* empty, so ``"=s"`` and ``"=s:"`` both
decode to ``("s", None)`` — or None when ``target`` does not start with
``=``: a backend-native id (``"@1"``, ``"%3"``, ``"w1:p1"``, ...) the caller
resolves itself. The window part is everything after the *first* ``:``;
that split is safe because bmad-loop mints window names
(``<kind>-<run_id>``) that never contain ``:``. Provided so a backend whose
native addressing differs decodes the grammar with one tested helper
instead of re-deriving it (see the herdr backend's ``_parse_target``)."""
if not target.startswith("="):
return None
session, _, window = target[1:].partition(":")
return (session, window or None)


class TerminalMultiplexer(ABC):
"""Transport backend for agent sessions: sessions, windows, and clients.

Expand All @@ -49,6 +67,21 @@ class TerminalMultiplexer(ABC):
``tui/launch.py``, ``probe.py``, and ``tui/data.py`` migrate onto it.
"""

# ------------------------------------------------------------ targets

def target(self, session: str, window: str | None = None) -> str:
"""Format the seam-canonical target token for ``session`` (optionally
one of its windows, *by name*). The default grammar is
``=session[:window]`` — historically tmux's exact-match syntax, now
owned by the seam: every target-taking method below accepts both this
token and the backend's native ids, and :func:`parse_target` is the
matching decoder. Backends MAY override to emit native ids, but the
result must stay a stable *by-name* reference: callers format targets
ahead of use (e.g. a parked window's return target), so eager
resolution to a live native id can go stale — keeping the token
symbolic and resolving lazily at use time is the recommended default."""
return f"={session}:{window}" if window else f"={session}"

# ----------------------------------------------------------- sessions

@abstractmethod
Expand Down Expand Up @@ -132,28 +165,33 @@ def window_alive(self, session: str, window_id: str) -> bool:
@abstractmethod
def kill_window(self, target: str) -> None:
"""Kill the targeted window (tolerant of it already being gone, and a
no-op on a transport failure)."""
no-op on a transport failure). ``target`` is a :meth:`target` token or
a backend-native window id."""

@abstractmethod
def select_window(self, target: str) -> None:
"""Make ``target`` the current window of its session (best-effort: a no-op
on a transport failure)."""
on a transport failure). ``target`` is a :meth:`target` token or a
backend-native window id."""

@abstractmethod
def set_window_option(self, target: str, option: str, value: str) -> None:
"""Set a user option on the targeted window (best-effort: a no-op on a
transport failure)."""
transport failure). ``target`` is a :meth:`target` token or a
backend-native window id."""

@abstractmethod
def unset_window_option(self, target: str, option: str) -> None:
"""Remove a user option from the targeted window (so a later read sees it
as unset, not as an empty value). Best-effort: a no-op on a transport
failure."""
failure. ``target`` is a :meth:`target` token or a backend-native
window id."""

@abstractmethod
def show_window_option(self, target: str, option: str) -> str:
"""Value of a user option on the targeted window ('' if unset, and '' on a
transport failure)."""
transport failure). ``target`` is a :meth:`target` token or a
backend-native window id."""

@abstractmethod
def pipe_pane(self, window_id: str, log_file: Path) -> None:
Expand All @@ -168,7 +206,9 @@ def send_text(self, window_id: str, text: str) -> None:

@abstractmethod
def attach_target_argv(self, target: str) -> list[str]:
"""argv that attaches the caller's terminal to ``target``."""
"""argv that attaches the caller's terminal to ``target`` (a
:meth:`target` token — session-only or session+window — or a
backend-native id)."""

@abstractmethod
def current_pane_id(self) -> str | None:
Expand All @@ -194,7 +234,8 @@ def detach_client(self) -> None:
def switch_client(self, target: str, last_fallback: bool = False) -> bool:
"""Switch the current client to ``target`` (optionally falling back to
the last client on failure). Returns True iff a switch happened — so a
transport failure returns False."""
transport failure returns False. ``target`` is a :meth:`target` token
or a backend-native id."""

@abstractmethod
def available(self) -> bool:
Expand Down
5 changes: 4 additions & 1 deletion src/bmad_loop/adapters/tmux_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,10 @@ class TmuxError(MultiplexerError):

class BaseTmuxBackend(TerminalMultiplexer):
"""tmux-family backend: all argv construction and every contract method, with
one overridable subprocess primitive (:meth:`_run`) every call funnels through."""
one overridable subprocess primitive (:meth:`_run`) every call funnels through.
The seam-canonical target grammar (``=session[:window]``, see
:meth:`TerminalMultiplexer.target`) coincides with tmux's exact-match target
syntax, so targets pass straight through to tmux — never parsed here."""

#: Output decoding for captured tmux text. ``None`` (POSIX) = locale default,
#: byte-identical to a bare ``text=True``; a Windows leaf sets ``"utf-8"``.
Expand Down
8 changes: 7 additions & 1 deletion src/bmad_loop/runs.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,14 @@ def attach_target_argv(target: str) -> list[str]:
return get_multiplexer().attach_target_argv(target)


def session_target(run_id: str) -> str:
"""Seam-canonical target token for the run's agent session (see
:meth:`TerminalMultiplexer.target`)."""
return get_multiplexer().target(session_name(run_id))


def attach_argv(run_id: str) -> list[str]:
return attach_target_argv(f"={session_name(run_id)}")
return attach_target_argv(session_target(run_id))


# ---------------------------------------------------- run resolution / liveness
Expand Down
8 changes: 4 additions & 4 deletions src/bmad_loop/tui/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,12 +360,12 @@ def action_attach(self) -> None:
if window is not None and (self._dashboard.decision_pending is not None or not agent_live):
launch.select_ctl_window(window)
self._attach_to_target(
f"={launch.CTL_SESSION}",
return_window=f"={launch.CTL_SESSION}:{window}",
launch.ctl_target(),
return_window=launch.ctl_target(window),
)
return
elif agent_live:
target = f"={session}"
target = runs.session_target(run_id)
else:
self.notify(
f"nothing to attach: no live agent session ({session}) and no "
Expand Down Expand Up @@ -459,7 +459,7 @@ def _launch_resolve(self, run_id: str) -> None:
self.notify("resolve launched but its window id was not captured", severity="error")
return
launch.select_ctl_window_id(win_id)
self._attach_to_target(f"={launch.CTL_SESSION}", return_window=win_id)
self._attach_to_target(launch.ctl_target(), return_window=win_id)

# -------------------------------------------------------- HITL pause review

Expand Down
14 changes: 10 additions & 4 deletions src/bmad_loop/tui/launch.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,16 @@ def ctl_window(run_id: str) -> str | None:
return None


def ctl_target(window: str | None = None) -> str:
"""Seam-canonical target token for the control session (optionally one of
its windows, by name); see :meth:`TerminalMultiplexer.target`."""
return get_multiplexer().target(CTL_SESSION, window)


def select_ctl_window(window: str) -> None:
"""Make `window` the control session's current window, so a plain attach
to the session lands on it (attach-session itself takes no window)."""
get_multiplexer().select_window(f"={CTL_SESSION}:{window}")
get_multiplexer().select_window(ctl_target(window))


def select_ctl_window_id(window_id: str) -> None:
Expand Down Expand Up @@ -162,9 +168,9 @@ def attach_plan(project: Path, run_id: str) -> tuple[list[str], str | None] | No
decision_pending(runs.run_dir_for(project, run_id)) or not agent_live
):
select_ctl_window(window)
return runs.attach_target_argv(f"={CTL_SESSION}"), f"={CTL_SESSION}:{window}"
return runs.attach_target_argv(ctl_target()), ctl_target(window)
if agent_live:
return runs.attach_target_argv(f"={session}"), None
return runs.attach_target_argv(runs.session_target(run_id)), None
return None


Expand All @@ -173,7 +179,7 @@ def kill_ctl_window(run_id: str) -> None:
if any. A no-op when the run was not launched from the TUI or tmux is gone."""
window = ctl_window(run_id)
if window is not None:
get_multiplexer().kill_window(f"={CTL_SESSION}:{window}")
get_multiplexer().kill_window(ctl_target(window))


def _ctl_window_candidates(project: Path) -> list[tuple[str, str]]:
Expand Down
54 changes: 53 additions & 1 deletion tests/test_multiplexer.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
from bmad_loop.adapters import tmux_base
from bmad_loop.adapters.base import SessionSpec
from bmad_loop.adapters.generic import GenericAdapter
from bmad_loop.adapters.multiplexer import MultiplexerError, TerminalMultiplexer
from bmad_loop.adapters.herdr_backend import HerdrMultiplexer
from bmad_loop.adapters.multiplexer import MultiplexerError, TerminalMultiplexer, parse_target
from bmad_loop.adapters.profile import get_profile
from bmad_loop.adapters.tmux_backend import TmuxMultiplexer
from bmad_loop.policy import LimitsPolicy, Policy
Expand Down Expand Up @@ -506,3 +507,54 @@ def test_dialect_leaf_new_window_routes_launch_through_hook(monkeypatch, tmp_pat
"wrapped:cmd",
]
assert "-e" not in rec.argv # env strategy fully delegated to the hook


# ------------------------------------------------------------ target contract
#
# target() is the seam-canonical encoder core uses instead of hand-assembling
# "=session[:window]" strings; parse_target is the matching decoder a native-id
# backend reuses instead of re-deriving the grammar. Pure string work: no
# subprocess, no env sensitivity, safe on every CI leg. Both backends are
# constructed directly (their constructors are documented side-effect-free).


def test_target_default_grammar():
mux = TmuxMultiplexer()
assert mux.target("s") == "=s"
assert mux.target("s", "w") == "=s:w"
# falsy window collapses to the session-only form, mirroring parse_target's
# "=s:" -> ("s", None) decode
assert mux.target("s", None) == "=s"
assert mux.target("s", "") == "=s"


def test_herdr_inherits_the_default_encoder():
# herdr resolves targets lazily at use time (_parse_target), so it must NOT
# override target() to eagerly emit native ids — a token formatted ahead of
# use (e.g. attach_plan's return_window) would go stale.
assert "target" not in HerdrMultiplexer.__dict__
assert HerdrMultiplexer().target("s", "w") == "=s:w"


@pytest.mark.parametrize(
("session", "window"),
[("s", None), ("s", "w"), ("bmad-loop-ctl", "run-20260714-abc")],
)
def test_parse_target_round_trips_the_encoder(session, window):
mux = TmuxMultiplexer()
assert parse_target(mux.target(session, window)) == (session, window)


def test_parse_target_edges():
# empty window part decodes like the session-only form
assert parse_target("=s:") == ("s", None)
# window is everything after the FIRST colon (minted names carry no colon,
# but the split rule is pinned regardless)
assert parse_target("=s:a:b") == ("s", "a:b")


@pytest.mark.parametrize("native", ["@1", "%3", "w1:p1"])
def test_parse_target_passes_native_ids_through(native):
# non-"=" targets are backend-native ids: the decoder answers None and the
# backend resolves them itself
assert parse_target(native) is None
Loading