From 01fcd03d9df73b6b8afb2ee0cb2fcf62348b8acd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Davor=20Raci=C4=87?= Date: Sun, 26 Jul 2026 20:22:48 +0200 Subject: [PATCH 1/6] fix(tui): report the parked-window return honestly (#227) return_attached_client() discarded switch_client's result and answered True unconditionally, so a failed switch plus a failed -l fallback still journaled the return and sent the sweep unattended while the human sat in the sweep window with their terminal never handed back. Thread the result out and clear RETURN_OPTION only on a real return, so a failed one is left for the parked window's trailer. Widen TerminalMultiplexer.detach_client from None to bool for the same reason: the returncode was in hand and dropped, so the detach branch carried the identical false positive. Closes #227 --- CHANGELOG.md | 11 +++++++++ docs/adapter-authoring-guide.md | 11 +++++---- src/bmad_loop/adapters/multiplexer.py | 13 ++++++++--- src/bmad_loop/adapters/tmux_base.py | 9 +++++--- src/bmad_loop/tui/launch.py | 30 +++++++++++++++++-------- tests/test_multiplexer.py | 2 +- tests/test_tui_launch.py | 32 +++++++++++++++++++++++++-- 7 files changed, 86 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4eb63d3..3db2e9b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -125,6 +125,17 @@ story `, the same annotation a sweep bundle writes. Both sprint and stories ### Fixed +- **`return_attached_client` no longer claims a return that never happened (#227).** It discarded + `switch_client`'s result and answered `True` unconditionally, so a failed switch plus a failed + `-l` fallback still journaled the return and sent the sweep unattended while the human sat in + the sweep window with their terminal never handed back. The result is now the return value, and + `RETURN_OPTION` is cleared only on a real return — a failed one is left for the parked window's + trailer, which is a second chance if someone dismisses the park prompt, not a rescue for an + unattended window. `TerminalMultiplexer.detach_client` widens from `None` to `bool` for the same + reason: it had the return code in hand and dropped it, so the detach branch carried the identical + false positive. Out-of-tree backends still returning `None` read as "nothing detached", which + keeps the option set — degraded, not broken. + - **Give psmux a working per-window option channel (#310).** psmux keeps one user-option scope per server and returns `''` for every `-w` read of an `@`-prefixed name, so both mechanisms riding per-window options were broken. The ctl-window project tag read the same bled value for diff --git a/docs/adapter-authoring-guide.md b/docs/adapter-authoring-guide.md index 9893a524..0f589a83 100644 --- a/docs/adapter-authoring-guide.md +++ b/docs/adapter-authoring-guide.md @@ -95,9 +95,11 @@ seams of a full OS port are in parked-window return hop, replayed opaquely by `switch_client` and the parked trailer; concrete default = the native pane id, so most backends inherit it — override only when your ids do not resolve from another session's context, - as psmux does to emit `=session:%N`), `detach_client`, `switch_client` (with - an optional last-client fallback), `available` (is this backend usable on - the current host). + as psmux does to emit `=session:%N`), `detach_client` / `switch_client` (with + an optional last-client fallback — both answer a bool the parked-window + return path trusts: True iff the backend reported the move happened, so a + backend with no real detach returns `False`, never a vacuous `True`), + `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 @@ -156,7 +158,8 @@ tee — runs a per-window **poller** thread that snapshots `pane read` into the whenever the content changes, which is exactly enough to drive the two log consumers a tmux tee would (`generic._log_activity_key`'s stall re-arm and `probe`'s marker discovery). Its module docstring is a **degradation ledger** of every such -divergence (sidecar options, poller `pipe_pane`, no-op `detach_client`, the attach +divergence (sidecar options, poller `pipe_pane`, the no-op `detach_client` — +which the widened seam now requires to answer `False`, not `None` — the attach argv, the advisory geometry, the protocol-version policy) — the reference for what "implement fresh" costs when the host has no tmux-shaped CLI. The operator-facing view — what a herdr _user_ notices and does — is diff --git a/src/bmad_loop/adapters/multiplexer.py b/src/bmad_loop/adapters/multiplexer.py index 7aaddf4e..de9ebc7c 100644 --- a/src/bmad_loop/adapters/multiplexer.py +++ b/src/bmad_loop/adapters/multiplexer.py @@ -277,9 +277,16 @@ def current_return_target(self) -> str | None: return self.current_pane_id() or None @abstractmethod - def detach_client(self) -> None: - """Detach the client viewing the current session (best-effort: a no-op on - a transport failure).""" + def detach_client(self) -> bool: + """Detach the client viewing the current session. Returns True iff the + backend reported a successful detach; a transport failure answers + False. On tmux that is "a client was actually detached" — the command + fails when nothing is attached — but a backend whose transport cannot + distinguish "detached nothing" from success answers True vacuously + (psmux does; see its module docstring). Callers that only want the + terminal handed back may ignore it; the parked-window return path + cannot, because clearing its return option on a detach that never + happened strands the human (see tui.launch.return_attached_client).""" @abstractmethod def switch_client(self, target: str, last_fallback: bool = False) -> bool: diff --git a/src/bmad_loop/adapters/tmux_base.py b/src/bmad_loop/adapters/tmux_base.py index dcfc2225..3ddca43d 100644 --- a/src/bmad_loop/adapters/tmux_base.py +++ b/src/bmad_loop/adapters/tmux_base.py @@ -429,11 +429,14 @@ def _display_message(self, fmt: str) -> str | None: return None return proc.stdout.strip() if proc.returncode == 0 else None - def detach_client(self) -> None: + def detach_client(self) -> bool: + # Returns True iff a client was detached; a transport failure didn't + # detach anything, so False is the honest answer. try: - self._run(["detach-client"], check=False) + proc = self._run(["detach-client"], check=False) except (subprocess.SubprocessError, OSError): - pass + return False + return proc.returncode == 0 def switch_client(self, target: str, last_fallback: bool = False) -> bool: # Returns True iff a switch happened; a transport failure didn't switch, so diff --git a/src/bmad_loop/tui/launch.py b/src/bmad_loop/tui/launch.py index b413ebad..53676a8d 100644 --- a/src/bmad_loop/tui/launch.py +++ b/src/bmad_loop/tui/launch.py @@ -121,10 +121,12 @@ def in_ctl_session() -> bool: return current_session() == CTL_SESSION -def detach_client() -> None: +def detach_client() -> bool: """Detach the tmux client viewing the current session, handing the terminal - back to the user. Processes in the session keep running.""" - get_multiplexer().detach_client() + back to the user. Processes in the session keep running. Returns True iff + the backend reported a successful detach (see + TerminalMultiplexer.detach_client for what each backend can promise).""" + return get_multiplexer().detach_client() def return_attached_client() -> bool: @@ -137,8 +139,17 @@ def return_attached_client() -> bool: psmux): switch that client back there (`-l` fallback if it's gone); - RETURN_DETACH: detach the client so a blocking `tmux attach` returns; - unset/empty: nobody attached with a return target — do nothing. - The option is then cleared so the post-exit return trailer doesn't fire a - second time. Returns True iff a client was actually returned.""" + Returns True iff a client was actually returned. The option is cleared only + then: a real return must not make the parked window's trailer fire a second + one, a failed return is left for the trailer to retry. That retry is a + second chance, not a rescue — new_parked_window parks on a blocking read + *before* the trailer, so it runs only once a human dismisses the park + prompt, never in the unattended case. On tmux both branches are equally + honest: a detach that found no client to detach fails the command, the + same non-return as a failed switch. psmux can report neither failure + (its detach/switch verbs report execution, not effect — see its module + docstring), but its inert option channel answers empty above before + either verb is reached.""" mux = get_multiplexer() if not mux_usable(mux): return False @@ -149,11 +160,12 @@ def return_attached_client() -> bool: if not ret: return False if ret == RETURN_DETACH: - mux.detach_client() + returned = mux.detach_client() else: - mux.switch_client(ret, last_fallback=True) - mux.unset_window_option(win, RETURN_OPTION) - return True + returned = mux.switch_client(ret, last_fallback=True) + if returned: + mux.unset_window_option(win, RETURN_OPTION) + return returned def decision_pending(run_dir: Path) -> bool: diff --git a/tests/test_multiplexer.py b/tests/test_multiplexer.py index 7286c5b4..2c07f192 100644 --- a/tests/test_multiplexer.py +++ b/tests/test_multiplexer.py @@ -239,11 +239,11 @@ def test_seam_methods_never_leak_raw_subprocess_error(boom_run, tmp_path): assert mux.show_window_option("@1", "opt") == "" assert mux.switch_client("s") is False assert mux.switch_client("s", last_fallback=True) is False + assert mux.detach_client() is False assert mux.kill_window("@1") is None assert mux.select_window("@1") is None assert mux.set_window_option("@1", "opt", "val") is None assert mux.unset_window_option("@1", "opt") is None - assert mux.detach_client() is None assert mux.pipe_pane("@1", tmp_path / "log") is None assert mux.window_pane_pids("@1") == [] diff --git a/tests/test_tui_launch.py b/tests/test_tui_launch.py index a488aae8..c2fa9b81 100644 --- a/tests/test_tui_launch.py +++ b/tests/test_tui_launch.py @@ -475,9 +475,12 @@ def test_detach_client_argv(fake_run): assert fake_run.calls == [["tmux", "detach-client"]] -def _return_fake(monkeypatch, *, win="@5", option="=main:%9", switch_rc=0): +def _return_fake( + monkeypatch, *, win="@5", option="=main:%9", switch_rc=0, fallback_rc=0, detach_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 -t -> switch_rc, + switch-client -l -> fallback_rc, detach-client -> detach_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") @@ -492,6 +495,10 @@ def fake(argv, **kwargs): out, rc = (f"{option}\n" if option else "", 0) elif verb == "switch-client" and argv[2] == "-t": out, rc = "", switch_rc + elif verb == "switch-client" and argv[2] == "-l": + out, rc = "", fallback_rc + elif verb == "detach-client": + out, rc = "", detach_rc else: out, rc = "", 0 return subprocess.CompletedProcess(argv, rc, stdout=out, stderr="") @@ -514,6 +521,27 @@ def test_return_attached_client_switch_fallback(monkeypatch): calls = _return_fake(monkeypatch, option="=main:%9", switch_rc=1) assert launch.return_attached_client() is True assert ["tmux", "switch-client", "-l"] in calls + # the fallback returned a client too, so the option is consumed — without + # this the unset could regress to primary-success-only and stay green + assert ["tmux", "set-option", "-wu", "-t", "@5", "@bmad_return_pane"] in calls + + +def test_return_attached_client_switch_fails_keeps_return_option(monkeypatch): + """Stale target plus no last client: nothing was returned, so say so — and + leave RETURN_OPTION set, or the post-exit trailer loses its retry.""" + calls = _return_fake(monkeypatch, option="=main:%9", switch_rc=1, fallback_rc=1) + assert launch.return_attached_client() is False + assert ["tmux", "switch-client", "-l"] in calls # fallback was attempted + assert not any(c[1] == "set-option" for c in calls) # option survives + + +def test_return_attached_client_detach_fails_keeps_return_option(monkeypatch): + """Nothing was attached to detach — same non-return as a failed switch, so + the option must survive here too.""" + calls = _return_fake(monkeypatch, option="detach", detach_rc=1) + assert launch.return_attached_client() is False + assert ["tmux", "detach-client"] in calls + assert not any(c[1] == "set-option" for c in calls) def test_return_attached_client_detaches(monkeypatch): From b2887090f3d4187e1813f381a5566a7b6b7e5339 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Davor=20Raci=C4=87?= Date: Sun, 26 Jul 2026 21:00:26 +0200 Subject: [PATCH 2/6] docs(adapters): correct the psmux switch-client reply claim (#227 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The psmux ledger entry said only `switch-client -t` carries a server reply. That describes psmux main, not the v3.3.7 the version gate admits: there the whole CLI arm is `send_control(cmd)?; return Ok(())`, so no form carries a reply and `-t` is exit-0-regardless too. The response path landed after the tag and only for `-t`, leaving `-l` unconditional either way. Understated the ceiling in the direction that matters, so state it at both revisions and name it as the rc-0 no-op family (#228). Also resolve the contradiction the same round introduced: the authoring guide promised the seam never answers a vacuous True while the ABC and the psmux ledger both documented psmux doing exactly that. Keep the requirement — report effect, not dispatch — and name psmux as the worked deviation, rather than relaxing the contract to "command succeeded", which is the failure #227 exists to remove. --- docs/adapter-authoring-guide.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/adapter-authoring-guide.md b/docs/adapter-authoring-guide.md index 0f589a83..c2b3f4cc 100644 --- a/docs/adapter-authoring-guide.md +++ b/docs/adapter-authoring-guide.md @@ -97,8 +97,13 @@ seams of a full OS port are in override only when your ids do not resolve from another session's context, as psmux does to emit `=session:%N`), `detach_client` / `switch_client` (with an optional last-client fallback — both answer a bool the parked-window - return path trusts: True iff the backend reported the move happened, so a - backend with no real detach returns `False`, never a vacuous `True`), + return path trusts: report **effect**, not that the command was dispatched, + so a backend with no real detach returns `False`. Report the closest thing + your transport can actually observe, and if it cannot observe effect at all, + say so in your degradation ledger rather than leaving the caller to assume: + psmux is the worked example — its CLI exits 0 whether or not a client moved, + so its booleans are vacuously `True` and the seam's honesty guarantee does + not hold there), `available` (is this backend usable on the current host). **Window targets.** The target-taking methods (`kill_window`, `select_window`, From 3a60c2959ecb5c8f236a61a7c4ec5b1402696f74 Mon Sep 17 00:00:00 2001 From: pbean Date: Sun, 26 Jul 2026 17:09:43 -0700 Subject: [PATCH 3/6] fix(sweep): tell a stranded client apart from an unreachable one (#227 review) Threading the boolean through fixes the switch branch and breaks the detach one. `sweep._return_after_decisions` consumes it to answer "can a human still answer here?", and the two failures answer that in opposite directions: a failed switch leaves the client in this window with someone in front of it, while a failed detach means there was no client to detach at all. Reporting both as a plain False keeps a --repeat sweep prompting into a window nobody is viewing, on input() that never returns - the livelock the function's own docstring exists to prevent, and the documented flow walks straight into it (tui-guide step 3 tells tmux users to detach right after answering; on herdr, whose detach is a no-op the widened seam now requires to answer False, it is not a race but the only outcome). So return_attached_client answers a ReturnOutcome instead: RETURNED, ATTENDED (keep talking to the terminal - nothing was recorded to return to, the backend is unusable, or the switch left the client here), UNREACHABLE (a detach found nothing attached, or the backend has no detach verb). RETURN_OPTION still clears only on RETURNED. The sweep goes unattended on anything but ATTENDED, and announces only a real return - UNREACHABLE journals sweep-return-no-client and prints nothing, because there is no one to read it. --- docs/tui-guide.md | 4 ++- src/bmad_loop/sweep.py | 27 ++++++++++---- src/bmad_loop/tui/launch.py | 70 +++++++++++++++++++++++++------------ tests/test_sweep.py | 53 +++++++++++++++++++--------- tests/test_tui_launch.py | 31 +++++++++------- 5 files changed, 126 insertions(+), 59 deletions(-) diff --git a/docs/tui-guide.md b/docs/tui-guide.md index 486120d1..dacb4668 100644 --- a/docs/tui-guide.md +++ b/docs/tui-guide.md @@ -481,7 +481,9 @@ toast. Then: 3. Detach with `ctrl-b d`. (On tmux this step is belt-and-suspenders — answering already hands your terminal back. On backends without a detach verb — e.g. the herdr adapter — the hand-back cannot be automatic: press its detach - chord, `ctrl+b q` on herdr.) + chord, `ctrl+b q` on herdr. Either way the sweep knows it can no longer reach + you and goes unattended for the rest of the run, so detaching first never + leaves it waiting on a prompt you can't see.) The banner clears on the next poll after the sweep journals anything further (the answer is recorded as a `decision:` line in `deferred-work.md`). Sweeps diff --git a/src/bmad_loop/sweep.py b/src/bmad_loop/sweep.py index 988d57b7..1edf1892 100644 --- a/src/bmad_loop/sweep.py +++ b/src/bmad_loop/sweep.py @@ -1006,16 +1006,31 @@ def _return_after_decisions(self) -> None: detach a plain-shell client, switch a tmux client back to its origin. A plain foreground sweep (nobody attached, no return target) is untouched. - After a successful return we go unattended for the rest of the run: a - later --repeat cycle's input() would otherwise block forever in a window - no one is viewing. New decisions then defer via the unattended path, - recorded for `bmad-loop decisions` or the next attended sweep.""" + We then go unattended for the rest of the run: a later --repeat cycle's + input() would otherwise block forever in a window no one is viewing. New + decisions defer via the unattended path instead, recorded for + `bmad-loop decisions` or the next attended sweep. + + The trigger for that is "nobody can answer here any more", NOT "the + hand-back succeeded" — the two come apart on a failed return, in + opposite directions. A failed switch leaves the client in this window + with a human in front of it (ATTENDED: keep prompting, which is the + whole point of #227). A failed detach means there was no client to + detach — nobody is watching, so going unattended is the only outcome + that does not strand a --repeat cycle on input(); the same holds for a + backend with no detach verb at all. Only a real return is announced: + UNREACHABLE prints nothing because there is no one to read it.""" from .tui import launch # import-light: launch.py has no textual imports - if launch.return_attached_client(): + outcome = launch.return_attached_client() + if outcome is launch.ReturnOutcome.ATTENDED: + return + self.prompting = False + if outcome is launch.ReturnOutcome.RETURNED: self.journal.append("sweep-returned-after-decisions") self.prompter.print_fn("✓ decisions recorded — sweep continues in the background") - self.prompting = False + else: + self.journal.append("sweep-return-no-client") def _apply_decision_effect(self, decision: Decision, option: DecisionOption) -> None: ledger = self.workspace.paths.deferred_work diff --git a/src/bmad_loop/tui/launch.py b/src/bmad_loop/tui/launch.py index 53676a8d..d09f6825 100644 --- a/src/bmad_loop/tui/launch.py +++ b/src/bmad_loop/tui/launch.py @@ -15,6 +15,7 @@ import re import subprocess import sys +from enum import StrEnum from pathlib import Path from .. import runs @@ -123,13 +124,35 @@ def in_ctl_session() -> bool: def detach_client() -> bool: """Detach the tmux client viewing the current session, handing the terminal - back to the user. Processes in the session keep running. Returns True iff - the backend reported a successful detach (see - TerminalMultiplexer.detach_client for what each backend can promise).""" + back to the user. Processes in the session keep running. Returns True iff a + client was actually detached — False both when the transport failed and when + there was nothing attached (see TerminalMultiplexer.detach_client for how + each backend establishes that).""" return get_multiplexer().detach_client() -def return_attached_client() -> bool: +class ReturnOutcome(StrEnum): + """What return_attached_client managed to do — and, for a caller that goes + unattended on the strength of it, whether a human can still answer here. + + A plain boolean cannot carry that: "the hand-back succeeded" and "there is + still someone at this terminal" are independent, and the two failures point + opposite ways. A failed *switch* leaves the client sitting in this very + window; a failed *detach* means there was no client to detach at all.""" + + RETURNED = "returned" + #: No hand-back, but a human may still be here: nothing was recorded to + #: return to (a plain foreground sweep), the backend is unusable, or the + #: switch failed with the client still in this window. The conservative + #: answer — a caller must keep talking to the terminal. + ATTENDED = "attended" + #: A hand-back was attempted and there was no client to hand back: the + #: detach found nothing attached, or the backend has no detach verb at all + #: (herdr). Nobody can answer a prompt in this window. + UNREACHABLE = "unreachable" + + +def return_attached_client() -> ReturnOutcome: """Hand an attached client back to its origin *now*, mid-process — the parked-window return move (see start_detached) executed while the window's command keeps running in the background, instead of after it exits. @@ -139,33 +162,36 @@ def return_attached_client() -> bool: psmux): switch that client back there (`-l` fallback if it's gone); - RETURN_DETACH: detach the client so a blocking `tmux attach` returns; - unset/empty: nobody attached with a return target — do nothing. - Returns True iff a client was actually returned. The option is cleared only - then: a real return must not make the parked window's trailer fire a second - one, a failed return is left for the trailer to retry. That retry is a - second chance, not a rescue — new_parked_window parks on a blocking read - *before* the trailer, so it runs only once a human dismisses the park - prompt, never in the unattended case. On tmux both branches are equally - honest: a detach that found no client to detach fails the command, the - same non-return as a failed switch. psmux can report neither failure - (its detach/switch verbs report execution, not effect — see its module - docstring), but its inert option channel answers empty above before - either verb is reached.""" + The option is cleared only on RETURNED: a real return must not make the + parked window's trailer fire a second one, a failed return is left for the + trailer to retry. That retry is a second chance, not a rescue — + new_parked_window parks on a blocking read *before* the trailer, so it runs + only once a human dismisses the park prompt, never in the unattended case. + + The two failures are not interchangeable, which is why this answers a + ReturnOutcome and not a bool. On tmux `detach-client` fails with "no + current client", so a failed detach is positive evidence that nobody is + watching (UNREACHABLE) — reporting it as the same non-return as a failed + switch would leave a --repeat sweep prompting into a window no one can + answer. A backend whose detach is a no-op (herdr) lands in the same place + for the same reason, which is what the seam's False-not-None rule buys it.""" mux = get_multiplexer() if not mux_usable(mux): - return False + return ReturnOutcome.ATTENDED win = mux.current_window_id() if win is None: - return False + return ReturnOutcome.ATTENDED ret = mux.show_window_option(win, RETURN_OPTION) if not ret: - return False + return ReturnOutcome.ATTENDED if ret == RETURN_DETACH: - returned = mux.detach_client() + outcome = ReturnOutcome.RETURNED if mux.detach_client() else ReturnOutcome.UNREACHABLE else: - returned = mux.switch_client(ret, last_fallback=True) - if returned: + switched = mux.switch_client(ret, last_fallback=True) + outcome = ReturnOutcome.RETURNED if switched else ReturnOutcome.ATTENDED + if outcome is ReturnOutcome.RETURNED: mux.unset_window_option(win, RETURN_OPTION) - return returned + return outcome def decision_pending(run_dir: Path) -> bool: diff --git a/tests/test_sweep.py b/tests/test_sweep.py index b77f97a0..8e0d1303 100644 --- a/tests/test_sweep.py +++ b/tests/test_sweep.py @@ -36,6 +36,7 @@ SweepPolicy, ) from bmad_loop.sweep import DecisionPrompter, SweepEngine, validate_migration, validate_triage +from bmad_loop.tui import launch from bmad_loop.verify import worktree_clean QUIET = NotifyPolicy(desktop=False, file=True) @@ -831,16 +832,17 @@ def _close_decision_plan(): ) -def test_interactive_decisions_return_client_goes_unattended(project, monkeypatch): - """When a client was attached to answer, the sweep hands the terminal back - after the decisions and goes unattended so later cycles don't block on a - detached window.""" - returned: list[bool] = [] +def _stub_return(monkeypatch, outcome): + """Pin return_attached_client's answer and record how often it was asked.""" + asked: list[object] = [] monkeypatch.setattr( "bmad_loop.tui.launch.return_attached_client", - lambda: bool(returned.append(True)) or True, + lambda: (asked.append(outcome), outcome)[1], ) - write_ledger(project, {"DW-1": "open"}) + return asked + + +def _run_one_decision_sweep(project): engine, _adapter = make_sweep( project, [triage_effect(_close_decision_plan())], @@ -849,7 +851,17 @@ def test_interactive_decisions_return_client_goes_unattended(project, monkeypatc ) summary = engine.run() assert not summary.paused - assert returned == [True] # asked exactly once, after the decisions phase + return engine + + +def test_interactive_decisions_return_client_goes_unattended(project, monkeypatch): + """When a client was attached to answer, the sweep hands the terminal back + after the decisions and goes unattended so later cycles don't block on a + detached window.""" + asked = _stub_return(monkeypatch, launch.ReturnOutcome.RETURNED) + write_ledger(project, {"DW-1": "open"}) + engine = _run_one_decision_sweep(project) + assert len(asked) == 1 # asked exactly once, after the decisions phase assert engine.prompting is False assert '"sweep-returned-after-decisions"' in journal_text(engine) @@ -857,20 +869,27 @@ def test_interactive_decisions_return_client_goes_unattended(project, monkeypatc def test_interactive_decisions_no_attach_stays_attended(project, monkeypatch): """A plain foreground sweep (nobody attached, no return target) keeps prompting and never emits the return event.""" - monkeypatch.setattr("bmad_loop.tui.launch.return_attached_client", lambda: False) + _stub_return(monkeypatch, launch.ReturnOutcome.ATTENDED) write_ledger(project, {"DW-1": "open"}) - engine, _adapter = make_sweep( - project, - [triage_effect(_close_decision_plan())], - answers=["1"], - prompting=True, - ) - summary = engine.run() - assert not summary.paused + engine = _run_one_decision_sweep(project) assert engine.prompting is True assert '"sweep-returned-after-decisions"' not in journal_text(engine) +def test_interactive_decisions_unreachable_goes_unattended(project, monkeypatch): + """A failed detach is not the same non-return as a failed switch: it means + there was no client to hand back, so nobody can answer here any more. The + sweep must go unattended anyway — keeping `prompting` would leave a + --repeat cycle blocked on input() in a window no one is viewing — but it + must not claim a hand-back that did not happen.""" + _stub_return(monkeypatch, launch.ReturnOutcome.UNREACHABLE) + write_ledger(project, {"DW-1": "open"}) + engine = _run_one_decision_sweep(project) + assert engine.prompting is False + assert '"sweep-return-no-client"' in journal_text(engine) + assert '"sweep-returned-after-decisions"' not in journal_text(engine) + + def test_unattended_skips_decisions(project): write_ledger(project, {"DW-1": "open", "DW-2": "open"}) plan = triage_result( diff --git a/tests/test_tui_launch.py b/tests/test_tui_launch.py index c2fa9b81..ef37ad8a 100644 --- a/tests/test_tui_launch.py +++ b/tests/test_tui_launch.py @@ -510,7 +510,7 @@ def fake(argv, **kwargs): def test_return_attached_client_switches_to_pane(monkeypatch): calls = _return_fake(monkeypatch, option="=main:%9") - assert launch.return_attached_client() is True + assert launch.return_attached_client() is launch.ReturnOutcome.RETURNED assert ["tmux", "switch-client", "-t", "=main:%9"] in calls assert ["tmux", "set-option", "-wu", "-t", "@5", "@bmad_return_pane"] in calls assert ["tmux", "switch-client", "-l"] not in calls # no fallback when -t works @@ -519,42 +519,47 @@ def test_return_attached_client_switches_to_pane(monkeypatch): def test_return_attached_client_switch_fallback(monkeypatch): calls = _return_fake(monkeypatch, option="=main:%9", switch_rc=1) - assert launch.return_attached_client() is True + assert launch.return_attached_client() is launch.ReturnOutcome.RETURNED assert ["tmux", "switch-client", "-l"] in calls # the fallback returned a client too, so the option is consumed — without # this the unset could regress to primary-success-only and stay green assert ["tmux", "set-option", "-wu", "-t", "@5", "@bmad_return_pane"] in calls -def test_return_attached_client_switch_fails_keeps_return_option(monkeypatch): - """Stale target plus no last client: nothing was returned, so say so — and - leave RETURN_OPTION set, or the post-exit trailer loses its retry.""" +def test_return_attached_client_switch_fails_stays_attended(monkeypatch): + """Stale target plus no last client: the client never left this window, so + the human is still in front of it — ATTENDED, and RETURN_OPTION stays set + or the post-exit trailer loses its retry.""" calls = _return_fake(monkeypatch, option="=main:%9", switch_rc=1, fallback_rc=1) - assert launch.return_attached_client() is False + assert launch.return_attached_client() is launch.ReturnOutcome.ATTENDED assert ["tmux", "switch-client", "-l"] in calls # fallback was attempted assert not any(c[1] == "set-option" for c in calls) # option survives -def test_return_attached_client_detach_fails_keeps_return_option(monkeypatch): - """Nothing was attached to detach — same non-return as a failed switch, so - the option must survive here too.""" +def test_return_attached_client_detach_fails_is_unreachable(monkeypatch): + """`detach-client` fails only when there is no current client, so a failed + detach is positive evidence that nobody is watching — the opposite of a + failed switch, and NOT the same answer. RETURN_OPTION still survives.""" calls = _return_fake(monkeypatch, option="detach", detach_rc=1) - assert launch.return_attached_client() is False + assert launch.return_attached_client() is launch.ReturnOutcome.UNREACHABLE assert ["tmux", "detach-client"] in calls assert not any(c[1] == "set-option" for c in calls) def test_return_attached_client_detaches(monkeypatch): calls = _return_fake(monkeypatch, option="detach") - assert launch.return_attached_client() is True + assert launch.return_attached_client() is launch.ReturnOutcome.RETURNED assert ["tmux", "detach-client"] in calls assert ["tmux", "set-option", "-wu", "-t", "@5", "@bmad_return_pane"] in calls assert not any(c[1] == "switch-client" for c in calls) def test_return_attached_client_noop_when_unset(monkeypatch): + """No return target recorded — a plain foreground sweep. Nothing was + attempted, so nothing can be concluded about who is at the terminal: the + conservative ATTENDED, never UNREACHABLE.""" calls = _return_fake(monkeypatch, option="") - assert launch.return_attached_client() is False + assert launch.return_attached_client() is launch.ReturnOutcome.ATTENDED assert not any(c[1] in ("switch-client", "detach-client", "set-option") for c in calls) @@ -568,7 +573,7 @@ def test_return_attached_client_noop_without_tmux(monkeypatch): ran: list = [] monkeypatch.setattr(tmux_base.shutil, "which", lambda name: None) monkeypatch.setattr(tmux_base.subprocess, "run", lambda *a, **k: ran.append(a)) - assert launch.return_attached_client() is False + assert launch.return_attached_client() is launch.ReturnOutcome.ATTENDED assert ran == [] # never shells out when tmux is missing From a0884f4420c70e1be60709a83df1a92ea250d216 Mon Sep 17 00:00:00 2001 From: pbean Date: Sun, 26 Jul 2026 17:09:54 -0700 Subject: [PATCH 4/6] fix(adapters): measure psmux client-verb effect instead of its exit code (#317) The parked-return path reads an empty option on psmux only while the per-window option channel is inert. #310 revived it, so the path now reaches switch_client/detach_client on Windows - and psmux answers both with an exit code that means nothing. At v3.3.7 every arm ends in `send_control(cmd)?; return Ok(())`: the only nonzero exit is an unreachable session server, so a detach succeeds with zero clients attached, and no form of switch-client - `-t` included - carries a server reply. Later builds narrow that only for `-t`. Taking those codes as the seam's booleans is the rc-0 no-op (#228) reached through Python, and it hands `return_attached_client` a True that clears the return option and reports a hand-back that never happened: #227's exact symptom, on the platform the psmux backend exists for. So discard the exit code and measure: count the clients attached to this session before and after the verb, answer on the drop. Never on an absolute count - a `-t` read against a session whose server is gone answers from whichever server the fallback picks, so "zero attached" alone proves nothing (#315); a drop needs two successful reads of the same session, the first nonzero. Unobservable answers False, never a vacuous True: the probe is self-detecting, since a build without #{session_attached} cannot echo a plausible integer. The switch leg reads as no effect today - it is inert at 3.3.7 (psmux/psmux#483) - and starts reporting True on its own when upstream lands the fix, which is why this measures rather than hardcoding. Also state the rule at the seam it belongs to: detach_client/switch_client owe effect, not dispatch, and the authoring guide now shows both ways to pay for it. --- CHANGELOG.md | 25 +++++-- docs/adapter-authoring-guide.md | 26 +++++-- src/bmad_loop/adapters/multiplexer.py | 28 ++++---- src/bmad_loop/adapters/psmux_backend.py | 85 ++++++++++++++++++++++- tests/test_psmux_backend.py | 90 +++++++++++++++++++++++++ 5 files changed, 227 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3db2e9b3..7fad34c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -128,13 +128,24 @@ story `, the same annotation a sweep bundle writes. Both sprint and stories - **`return_attached_client` no longer claims a return that never happened (#227).** It discarded `switch_client`'s result and answered `True` unconditionally, so a failed switch plus a failed `-l` fallback still journaled the return and sent the sweep unattended while the human sat in - the sweep window with their terminal never handed back. The result is now the return value, and - `RETURN_OPTION` is cleared only on a real return — a failed one is left for the parked window's - trailer, which is a second chance if someone dismisses the park prompt, not a rescue for an - unattended window. `TerminalMultiplexer.detach_client` widens from `None` to `bool` for the same - reason: it had the return code in hand and dropped it, so the detach branch carried the identical - false positive. Out-of-tree backends still returning `None` read as "nothing detached", which - keeps the option set — degraded, not broken. + the sweep window with their terminal never handed back. `RETURN_OPTION` is now cleared only on a + real return — a failed one is left for the parked window's trailer, which is a second chance if + someone dismisses the park prompt, not a rescue for an unattended window. + + The two failures are reported apart, because they point opposite ways: a failed switch leaves + the client in this window with a human in front of it (`ATTENDED` — an attended sweep keeps + prompting, which is the fix), while a failed detach means there was no client to detach at all + (`UNREACHABLE` — the sweep goes unattended, or a `--repeat` cycle blocks forever on a prompt no + one can see). Announcing it stays tied to a real return; `UNREACHABLE` journals + `sweep-return-no-client` and prints nothing. + + `TerminalMultiplexer.detach_client` widens from `None` to `bool` to carry that, and both client + verbs now owe **effect, not dispatch**. tmux reads it off the exit code. psmux cannot — every arm + of its `detach-client` / `switch-client` exits 0 whether or not a client moved — so it measures + the session's attached-client count across the call and answers on the drop; unobservable + degrades to `False`, never a vacuous `True` (#317). That path went live with the option channel + in #310, so on Windows the return was reachable and dishonest at the same time. Out-of-tree + backends still returning `None` read as "nothing detached" — degraded, not broken. - **Give psmux a working per-window option channel (#310).** psmux keeps one user-option scope per server and returns `''` for every `-w` read of an `@`-prefixed name, so both mechanisms diff --git a/docs/adapter-authoring-guide.md b/docs/adapter-authoring-guide.md index c2b3f4cc..7c5b4630 100644 --- a/docs/adapter-authoring-guide.md +++ b/docs/adapter-authoring-guide.md @@ -98,14 +98,24 @@ seams of a full OS port are in as psmux does to emit `=session:%N`), `detach_client` / `switch_client` (with an optional last-client fallback — both answer a bool the parked-window return path trusts: report **effect**, not that the command was dispatched, - so a backend with no real detach returns `False`. Report the closest thing - your transport can actually observe, and if it cannot observe effect at all, - say so in your degradation ledger rather than leaving the caller to assume: - psmux is the worked example — its CLI exits 0 whether or not a client moved, - so its booleans are vacuously `True` and the seam's honesty guarantee does - not hold there), + so a backend with no real detach returns `False`. If your CLI's exit code + already means "a client moved" you are done (tmux: `detach-client` fails with + _no current client_). If it does not, **measure** — psmux is the worked + example: every arm of its `detach-client` / `switch-client` exits 0 whether or + not a client moved, so the backend counts the session's attached clients + across the call and answers on the drop. Where even that is unavailable, + answer `False` and record the gap in your degradation ledger; a vacuous `True` + is the one answer that strands a human), `available` (is this backend usable on the current host). + The caller's two failures are not symmetric, which is what makes the rule + worth the round-trip: a failed `switch_client` leaves the client in the window + it was already in, while a failed `detach_client` means there was no client + there at all. `tui.launch.return_attached_client` reports those as `ATTENDED` + and `UNREACHABLE`, and an attended sweep keeps prompting on the first but goes + unattended on the second. Answering `True` when nothing happened collapses + both into "the human has their terminal back", which is #227. + **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 @@ -164,7 +174,9 @@ whenever the content changes, which is exactly enough to drive the two log consu a tmux tee would (`generic._log_activity_key`'s stall re-arm and `probe`'s marker discovery). Its module docstring is a **degradation ledger** of every such divergence (sidecar options, poller `pipe_pane`, the no-op `detach_client` — -which the widened seam now requires to answer `False`, not `None` — the attach +which the widened seam now requires to answer `False`, not `None`, so the +parked-return path reads it as `UNREACHABLE` and an attended sweep stops +prompting into a window whose client only a manual chord can release — the attach argv, the advisory geometry, the protocol-version policy) — the reference for what "implement fresh" costs when the host has no tmux-shaped CLI. The operator-facing view — what a herdr _user_ notices and does — is diff --git a/src/bmad_loop/adapters/multiplexer.py b/src/bmad_loop/adapters/multiplexer.py index de9ebc7c..44005226 100644 --- a/src/bmad_loop/adapters/multiplexer.py +++ b/src/bmad_loop/adapters/multiplexer.py @@ -278,22 +278,26 @@ def current_return_target(self) -> str | None: @abstractmethod def detach_client(self) -> bool: - """Detach the client viewing the current session. Returns True iff the - backend reported a successful detach; a transport failure answers - False. On tmux that is "a client was actually detached" — the command - fails when nothing is attached — but a backend whose transport cannot - distinguish "detached nothing" from success answers True vacuously - (psmux does; see its module docstring). Callers that only want the - terminal handed back may ignore it; the parked-window return path - cannot, because clearing its return option on a detach that never - happened strands the human (see tui.launch.return_attached_client).""" + """Detach the client viewing the current session. Returns True iff a + client was actually detached — **effect, not dispatch**: a transport + failure answers False, and so does a backend with no real detach. + tmux gets this from the exit code (`detach-client` fails with "no + current client"); a backend whose CLI exits 0 either way measures it + instead (psmux counts the session's attached clients across the call). + Callers that only want the terminal handed back may ignore the answer; + the parked-window return path cannot — it clears its return option on a + True, and a vacuous one strands the human, while a False is positive + evidence that nobody is watching this window any more (see + tui.launch.return_attached_client).""" @abstractmethod 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. ``target`` is a :meth:`target` token - or a backend-native id.""" + the last client on failure). Returns True iff a switch happened — the + same effect-not-dispatch rule as :meth:`detach_client`, so a transport + failure returns False and a backend whose CLI cannot report the move + measures it or answers False. ``target`` is a :meth:`target` token or a + backend-native id.""" @abstractmethod def available(self) -> bool: diff --git a/src/bmad_loop/adapters/psmux_backend.py b/src/bmad_loop/adapters/psmux_backend.py index aecadf47..8a501f32 100644 --- a/src/bmad_loop/adapters/psmux_backend.py +++ b/src/bmad_loop/adapters/psmux_backend.py @@ -25,7 +25,11 @@ the window-option verbs, the ``@``-prefixed columns of ``list_windows`` and the parked trailer route through a substitute channel — see the ``per-window option channel (#310)`` block below for the model and its -rules. ``available()`` additionally gates on +rules. ``detach-client`` and ``switch-client`` report dispatch rather than +effect — every arm exits 0 whether or not a client moved — so both seam +booleans are measured against the session's attached-client count instead of +read off the exit code; see the ``client verbs: observed effect (#317)`` +block. ``available()`` additionally gates on the reported version: psmux releases up to 3.3.6 kill recycled PIDs during pane/session teardown without a process-identity check, which can take down an unrelated long-lived process mid-run. The psmux behaviors cited in this @@ -721,6 +725,85 @@ def current_return_target(self) -> str | None: return pane return self.target(session, pane) + # ------------------------------- client verbs: observed effect (#317) + # + # psmux's client verbs report *dispatch*, not effect. At ``v3.3.7`` both CLI + # arms end in ``send_control(cmd)?; return Ok(())``, so the only nonzero exit + # is an unreachable session server: ``detach-client`` succeeds with zero + # clients attached (and a flag-less detach is promoted server-side to + # detach-all), and *no* form of ``switch-client`` — ``-t`` included — + # carries a server reply. Later builds narrow this but do not close it: a + # response path landed after the tag and only for ``-t``, leaving ``-l`` + # exit-0 regardless. Taking those exit codes as the seam's booleans is the + # rc-0 no-op (#228) reached through Python instead of a shell fallback, and + # it is what ``tui.launch.return_attached_client`` would consume to decide a + # human has been handed their terminal back. + # + # So the exit code is discarded and the effect is measured: count the + # clients attached to this session before and after the verb, and answer on + # the DELTA. Never on an absolute count — a ``-t`` read against a session + # whose server is gone answers from whichever server the fallback picks, so + # "zero attached" on its own proves nothing (#315). A drop cannot be + # manufactured that way: it needs two successful reads of the same session, + # the first of them nonzero. + # + # Unobservable answers False, never a vacuous True. For the detach that is + # safe in both directions — the caller's UNREACHABLE and RETURNED agree that + # nobody is left at this terminal, and the only cost is the parked trailer + # re-issuing a detach that no-ops. For the switch it is also the truth + # today: the switch leg is inert at 3.3.7 (psmux/psmux#483), so no client + # ever moves. When upstream lands that fix the probe reports it without a + # code change here — which is why this measures rather than hardcoding. + # + # Residue: a switch whose target pane lives in THIS session moves the client + # between windows without changing the session's attached count, so it reads + # as no effect. Reachable only when the return target was recorded from + # inside a control-session window; the caller then keeps prompting, which is + # the safe direction. + + def _attached_clients(self, session: str) -> int | None: + """Clients attached to ``session``, or None when psmux cannot say. + + Self-detecting on purpose: an unsupported format field cannot answer a + plausible integer, so a psmux build that does not carry + ``#{session_attached}`` degrades to None instead of a wrong count. + """ + try: + proc = self._run( + ["display-message", "-p", "-t", session, "#{session_attached}"], check=False + ) + except (subprocess.SubprocessError, OSError): + return None + if proc.returncode != 0: + return None + text = proc.stdout.strip() + return int(text) if text.isdigit() else None + + def _client_left(self, verb: list[str]) -> bool: + """Run a client verb and answer whether a client left this session.""" + session = self.current_session() + if not session: + # Not inside a pane (or the probe failed): there is no "this + # session" to measure against, and no client of ours to move. + return False + before = self._attached_clients(session) + try: + self._run(verb, check=False) + except (subprocess.SubprocessError, OSError): + return False + after = self._attached_clients(session) + if before is None or after is None: + return False + return before > 0 and after < before + + def detach_client(self) -> bool: + return self._client_left(["detach-client"]) + + def switch_client(self, target: str, last_fallback: bool = False) -> bool: + if self._client_left(["switch-client", "-t", target]): + return True + return last_fallback and self._client_left(["switch-client", "-l"]) + def pipe_pane(self, window_id: str, log_file: Path) -> None: # The base's POSIX `cat >>` sink assumes a POSIX host shell, and psmux # strips every dash-flag token from the piped command before spawning it diff --git a/tests/test_psmux_backend.py b/tests/test_psmux_backend.py index d73a9824..5d1dcccc 100644 --- a/tests/test_psmux_backend.py +++ b/tests/test_psmux_backend.py @@ -1227,3 +1227,93 @@ def fake(argv, **kwargs): candidates = launch._ctl_window_candidates(tmp_path) assert candidates == [("bmad-loop-ctl:@2", "run-20260726-1")] + + +# ---------------------------------------- client verbs: observed effect (#317) + + +def _client_fake(monkeypatch, *, attached, session="ctl", verb_rc=0): + """Script the two probes the client verbs measure with. + + ``attached`` is the successive answers to ``#{session_attached}``, consumed + in call order — so a detach that worked is ["1", "0"] and psmux's rc-0 no-op + is ["1", "1"]. Every verb exits ``verb_rc`` (0 by default: on psmux the exit + code says nothing, which is the whole point). + """ + monkeypatch.setenv("TMUX", "/tmp/psmux-1000/default,123,0") # inside a pane + counts = list(attached) + calls: list[list] = [] + + def fake(argv, **kwargs): + calls.append(list(argv)) + if argv[1] == "display-message" and argv[-1] == "#{session_name}": + return subprocess.CompletedProcess(argv, 0, stdout=f"{session}\n", stderr="") + if argv[1] == "display-message" and argv[-1] == "#{session_attached}": + return subprocess.CompletedProcess(argv, 0, stdout=f"{counts.pop(0)}\n", stderr="") + return subprocess.CompletedProcess(argv, verb_rc, stdout="", stderr="") + + monkeypatch.setattr(tmux_base.subprocess, "run", fake) + return calls + + +def test_psmux_detach_reports_the_observed_drop(monkeypatch): + calls = _client_fake(monkeypatch, attached=["1", "0"]) + assert PsmuxMultiplexer().detach_client() is True + assert ["psmux", "detach-client"] in calls + # both probes routed to the session, never left to the most-recent fallback + assert calls.count(["psmux", "display-message", "-p", "-t", "ctl", "#{session_attached}"]) == 2 + + +def test_psmux_detach_with_nothing_attached_is_false(monkeypatch): + """psmux's detach-client exits 0 with zero clients attached, so the exit + code cannot answer this — the count has to. Reading rc here is exactly the + vacuous True that strands the human (#317).""" + calls = _client_fake(monkeypatch, attached=["0", "0"]) + assert PsmuxMultiplexer().detach_client() is False + assert ["psmux", "detach-client"] in calls # attempted, just not effective + + +def test_psmux_client_verbs_never_claim_an_unobservable_effect(monkeypatch): + """A psmux build that does not carry #{session_attached} echoes something + non-numeric; that degrades to False, never to a vacuous True.""" + _client_fake(monkeypatch, attached=["#{session_attached}", "#{session_attached}"]) + assert PsmuxMultiplexer().detach_client() is False + + +def test_psmux_detach_outside_a_pane_is_false(monkeypatch): + """No TMUX, so current_session answers None: there is no session to measure + and no client of ours to move. Answer False without issuing a flag-less + detach, which psmux promotes server-side to detach-all.""" + calls = _client_fake(monkeypatch, attached=[]) + monkeypatch.delenv("TMUX", raising=False) + assert PsmuxMultiplexer().detach_client() is False + assert not any(c[1] == "detach-client" for c in calls) + + +def test_psmux_switch_reports_the_drop(monkeypatch): + calls = _client_fake(monkeypatch, attached=["1", "0"]) + assert PsmuxMultiplexer().switch_client("ctl:%9") is True + assert ["psmux", "switch-client", "-t", "ctl:%9"] in calls + assert not any(c[2:] == ["-l"] for c in calls) # no fallback when -t moved it + + +def test_psmux_switch_falls_back_then_reports_the_drop(monkeypatch): + calls = _client_fake(monkeypatch, attached=["1", "1", "1", "0"]) + assert PsmuxMultiplexer().switch_client("ctl:%9", last_fallback=True) is True + assert ["psmux", "switch-client", "-l"] in calls + + +def test_psmux_switch_is_false_while_the_leg_is_inert(monkeypatch): + """psmux/psmux#483: the switch moves no client, and every arm still exits 0. + Both legs are attempted and both read as no effect, so the caller keeps + prompting instead of being told the human was handed their terminal back.""" + calls = _client_fake(monkeypatch, attached=["1", "1", "1", "1"]) + assert PsmuxMultiplexer().switch_client("ctl:%9", last_fallback=True) is False + assert ["psmux", "switch-client", "-t", "ctl:%9"] in calls + assert ["psmux", "switch-client", "-l"] in calls + + +def test_psmux_switch_without_fallback_does_not_attempt_it(monkeypatch): + calls = _client_fake(monkeypatch, attached=["1", "1"]) + assert PsmuxMultiplexer().switch_client("ctl:%9") is False + assert not any(c[1:3] == ["switch-client", "-l"] for c in calls) From 04a65ea636679c9533f17294a41a4a42b9713df2 Mon Sep 17 00:00:00 2001 From: pbean Date: Sun, 26 Jul 2026 17:11:26 -0700 Subject: [PATCH 5/6] test(adapters): make the psmux out-of-pane guard fail on its assertion The case scripted no attached-client counts, so deleting the guard failed the test on the fixture running dry rather than on the behavior. Script counts a probe could read: the ablation now lands on `assert ... is False`. --- tests/test_psmux_backend.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_psmux_backend.py b/tests/test_psmux_backend.py index 5d1dcccc..a34e419e 100644 --- a/tests/test_psmux_backend.py +++ b/tests/test_psmux_backend.py @@ -1284,7 +1284,9 @@ def test_psmux_detach_outside_a_pane_is_false(monkeypatch): """No TMUX, so current_session answers None: there is no session to measure and no client of ours to move. Answer False without issuing a flag-less detach, which psmux promotes server-side to detach-all.""" - calls = _client_fake(monkeypatch, attached=[]) + # Scripted counts a probe COULD read, so removing the guard fails this on + # its assertions rather than on the fixture running dry. + calls = _client_fake(monkeypatch, attached=["1", "0"]) monkeypatch.delenv("TMUX", raising=False) assert PsmuxMultiplexer().detach_client() is False assert not any(c[1] == "detach-client" for c in calls) From 740986cbaec241d5578d117c65a5354b5dac43de Mon Sep 17 00:00:00 2001 From: pbean Date: Sun, 26 Jul 2026 17:40:45 -0700 Subject: [PATCH 6/6] docs(adapters): scope the client-verb effect claims (#227 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two places where the prose claimed more than the seam delivers — the same defect class already fixed once in c262708. The psmux worked example described the attached-client count as if it proved any switch. It proves a detach, and the switch the return path normally performs (the recorded target is a pane in the session the client came from, so a real switch leaves this session) — but not a switch within this session, which moves the client between windows without moving the count. psmux_backend.py already carried that under `Residue:`; the guide did not. Failing that way is conformant, so the rule is unchanged: the caller reads no effect and keeps prompting. "A failed detach_client means there was no client there at all" is true on tmux and nowhere else. The same section already mandates False for an unverifiable effect, and the guide's own herdr paragraph describes a no-op detach whose client only a manual chord releases. False means "no verified hand-back"; UNREACHABLE is the caller's policy for that uncertainty, justified by the asymmetric cost — prompting into an unwatched window blocks a --repeat cycle on input() forever, while going unattended in front of a human only defers decisions to `bmad-loop decisions`. launch.py and sweep.py repeated the same overstatement, so they move with the guide rather than leaving the inconsistency relocated. No behavior change; the no-drop-reads-False shape is already covered by tests/test_psmux_backend.py. --- docs/adapter-authoring-guide.md | 31 +++++++++++++++++++++++-------- src/bmad_loop/sweep.py | 20 +++++++++++--------- src/bmad_loop/tui/launch.py | 29 +++++++++++++++++++---------- 3 files changed, 53 insertions(+), 27 deletions(-) diff --git a/docs/adapter-authoring-guide.md b/docs/adapter-authoring-guide.md index 7c5b4630..36fe4f3b 100644 --- a/docs/adapter-authoring-guide.md +++ b/docs/adapter-authoring-guide.md @@ -103,18 +103,33 @@ seams of a full OS port are in _no current client_). If it does not, **measure** — psmux is the worked example: every arm of its `detach-client` / `switch-client` exits 0 whether or not a client moved, so the backend counts the session's attached clients - across the call and answers on the drop. Where even that is unavailable, - answer `False` and record the gap in your degradation ledger; a vacuous `True` - is the one answer that strands a human), + across the call and answers on the drop. Match the measure to what the verb is + meant to change, and record what it cannot see: that count reaches a detach, + and reaches the switch the return path normally performs — the recorded target + is a pane in the session the client came _from_, so a real switch leaves this + session — but a switch _within_ this session moves the client between windows + without moving the count, and so reads as no effect. Failing that way is + conformant: the caller keeps prompting, which is the safe direction. psmux + carries exactly that blind spot, in its `Residue:` note. Where even a measure + is unavailable, answer `False` and record the gap in your degradation ledger; + a vacuous `True` is the one answer that strands a human), `available` (is this backend usable on the current host). The caller's two failures are not symmetric, which is what makes the rule - worth the round-trip: a failed `switch_client` leaves the client in the window - it was already in, while a failed `detach_client` means there was no client - there at all. `tui.launch.return_attached_client` reports those as `ATTENDED` + worth the round-trip. A failed `switch_client` is positive evidence: the + client is still in the window it was already in, so someone may well be + watching. A failed `detach_client` is not evidence of anything — `False` there + means only "no verified hand-back", and covers all three of nothing was + attached (tmux says so outright), the effect could not be observed, and the + backend has no detach verb at all (herdr, whose client only a manual chord + releases). `tui.launch.return_attached_client` reports the two as `ATTENDED` and `UNREACHABLE`, and an attended sweep keeps prompting on the first but goes - unattended on the second. Answering `True` when nothing happened collapses - both into "the human has their terminal back", which is #227. + unattended on the second. `UNREACHABLE` is therefore the caller's _policy_ for + an unverified detach rather than a claim the window is empty: assuming a human + is still there costs a `--repeat` cycle blocked forever on `input()` in a + window nobody may be viewing, which is the worse way to be wrong. What no + backend may do is answer `True` when nothing happened — that collapses both + failures into "the human has their terminal back", which is #227. **Window targets.** The target-taking methods (`kill_window`, `select_window`, the window-option trio, `attach_target_argv`, `switch_client`) receive one of two diff --git a/src/bmad_loop/sweep.py b/src/bmad_loop/sweep.py index 1edf1892..41feda0a 100644 --- a/src/bmad_loop/sweep.py +++ b/src/bmad_loop/sweep.py @@ -1011,15 +1011,17 @@ def _return_after_decisions(self) -> None: decisions defer via the unattended path instead, recorded for `bmad-loop decisions` or the next attended sweep. - The trigger for that is "nobody can answer here any more", NOT "the - hand-back succeeded" — the two come apart on a failed return, in - opposite directions. A failed switch leaves the client in this window - with a human in front of it (ATTENDED: keep prompting, which is the - whole point of #227). A failed detach means there was no client to - detach — nobody is watching, so going unattended is the only outcome - that does not strand a --repeat cycle on input(); the same holds for a - backend with no detach verb at all. Only a real return is announced: - UNREACHABLE prints nothing because there is no one to read it.""" + The trigger for that is "nobody can be relied on to answer here any + more", NOT "the hand-back succeeded" — the two come apart on a failed + return, in opposite directions. A failed switch is evidence the client + is still in this window with a human in front of it (ATTENDED: keep + prompting, which is the whole point of #227). A failed detach reports + only that no hand-back was verified — nothing attached, an effect the + backend cannot observe, or no detach verb at all — and under that + uncertainty going unattended is the outcome that does not strand a + --repeat cycle on input(); the decisions it defers stay reachable via + `bmad-loop decisions`. Only a real return is announced: UNREACHABLE + prints nothing, since there may be no one to read it.""" from .tui import launch # import-light: launch.py has no textual imports outcome = launch.return_attached_client() diff --git a/src/bmad_loop/tui/launch.py b/src/bmad_loop/tui/launch.py index d09f6825..4ace4f2a 100644 --- a/src/bmad_loop/tui/launch.py +++ b/src/bmad_loop/tui/launch.py @@ -138,7 +138,8 @@ class ReturnOutcome(StrEnum): A plain boolean cannot carry that: "the hand-back succeeded" and "there is still someone at this terminal" are independent, and the two failures point opposite ways. A failed *switch* leaves the client sitting in this very - window; a failed *detach* means there was no client to detach at all.""" + window; a failed *detach* reports no verified hand-back, which is not the + same claim and does not license the same response.""" RETURNED = "returned" #: No hand-back, but a human may still be here: nothing was recorded to @@ -146,9 +147,12 @@ class ReturnOutcome(StrEnum): #: switch failed with the client still in this window. The conservative #: answer — a caller must keep talking to the terminal. ATTENDED = "attended" - #: A hand-back was attempted and there was no client to hand back: the - #: detach found nothing attached, or the backend has no detach verb at all - #: (herdr). Nobody can answer a prompt in this window. + #: A hand-back was attempted and did not verifiably happen: the detach found + #: nothing attached, the effect could not be observed, or the backend has no + #: detach verb at all (herdr). A caller must not rely on anyone answering a + #: prompt in this window — a policy for the uncertainty, not a proof that + #: the window is empty (see return_attached_client for why it is the safe + #: way to be wrong). UNREACHABLE = "unreachable" @@ -169,12 +173,17 @@ def return_attached_client() -> ReturnOutcome: only once a human dismisses the park prompt, never in the unattended case. The two failures are not interchangeable, which is why this answers a - ReturnOutcome and not a bool. On tmux `detach-client` fails with "no - current client", so a failed detach is positive evidence that nobody is - watching (UNREACHABLE) — reporting it as the same non-return as a failed - switch would leave a --repeat sweep prompting into a window no one can - answer. A backend whose detach is a no-op (herdr) lands in the same place - for the same reason, which is what the seam's False-not-None rule buys it.""" + ReturnOutcome and not a bool. A failed switch is positive evidence that the + client is still in this window, so ATTENDED keeps the caller prompting. A + failed detach carries no such evidence in general: on tmux it does + (`detach-client` fails with "no current client"), but off tmux False also + covers an effect the backend could not observe and a detach verb it does + not have at all — herdr, whose False rather than None is exactly what the + seam's widened return type buys. UNREACHABLE is the policy for all three, + because the two ways of being wrong are not equally bad: prompting into a + window no one is viewing blocks a --repeat sweep on input() forever, while + going unattended in front of a human only defers this cycle's decisions to + `bmad-loop decisions` or the next attended sweep.""" mux = get_multiplexer() if not mux_usable(mux): return ReturnOutcome.ATTENDED