diff --git a/CHANGELOG.md b/CHANGELOG.md index b711d728..b25d079a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -180,6 +180,7 @@ breaking changes may land in a minor release. ### Fixed +- Tell a failed window listing apart from an empty session: liveness raises when the failure is unproven, metadata keeps its sentinel and warns (#525). - Anchor the TUI's paused-spec read and its `Request replan` write on the tree the run owns. Under isolation both resolved against the main checkout, so the review modals showed that copy of the spec and the replan reset it — reporting success while the run's diff --git a/src/bmad_loop/adapters/multiplexer.py b/src/bmad_loop/adapters/multiplexer.py index 9aa60036..1325343b 100644 --- a/src/bmad_loop/adapters/multiplexer.py +++ b/src/bmad_loop/adapters/multiplexer.py @@ -216,17 +216,30 @@ def list_window_ids(self, session: str) -> list[str]: that diverges remains usable, but falls back to the ambiguous by-name lookup whenever several kinds share a run id. - Raises :class:`MultiplexerError` if the transport itself fails (timeout / - missing binary): an empty list means "no windows" and must not be - conflated with "couldn't ask" — this op backs the engine's liveness - probe (:meth:`window_alive`).""" + Raises :class:`MultiplexerError` whenever the listing could not be TAKEN + — a transport failure (timeout / missing binary) or a query that failed + without proving the session gone: an empty list means "no windows" and + must not be conflated with "couldn't ask", because this op backs the + engine's liveness probe (:meth:`window_alive`). + + So ``[]`` carries a positive claim, not a shrug: the backend either + listed the session's windows and found none, or established that the + session no longer exists. A backend answering over a server therefore + owes callers a discrimination — a server that errors while its windows + are alive must not answer ``[]`` (#525). Which conditions PROVE absence + is the backend's own question: the exit code alone does not decide it, + and neither does a confirming :meth:`has_session`, whose False is + weaker than it looks (see its note).""" @abstractmethod def list_windows(self, session: str, fields: list[str]) -> list[tuple[str, ...]]: """One tuple per window in ``session``, each holding the requested backend fields in order. Best-effort: returns ``[]`` on a transport - failure (unlike :meth:`list_window_ids`, this is metadata, not a liveness - probe, so a sentinel is safe). + failure OR a failed query (unlike :meth:`list_window_ids`, this is + metadata, not a liveness probe, so a sentinel is safe — the answer + degrades toward doing nothing, never toward claiming a death or a + kill). A backend SHOULD still say on stderr when the failure did not + prove the session gone, so an every-call failure is not silent (#525). A ``window_id`` column carries the same id form :meth:`current_window_id` AND :meth:`list_window_ids` return; core compares all three directly. The @@ -241,10 +254,13 @@ def list_windows(self, session: str, fields: list[str]) -> list[tuple[str, ...]] def window_alive(self, session: str, window_id: str) -> bool: """True iff ``window_id`` is still a window of ``session``. - May raise :class:`MultiplexerError` when liveness is unknowable (a - transport timeout / missing binary) — callers must treat that as "don't - know", not "dead", and must not tear down a possibly-working session on - it.""" + May raise :class:`MultiplexerError` when liveness is unknowable — a + transport timeout / missing binary, or any other failure to take the + listing this membership test reads (see :meth:`list_window_ids`). + Callers must treat that as "don't know", not "dead", and must not tear + down a possibly-working session on it. Reachable in ordinary operation, + not only under a hung binary: a live server that refuses or drops the + query answers here (#525).""" @abstractmethod def kill_window(self, target: str) -> None: diff --git a/src/bmad_loop/adapters/psmux_backend.py b/src/bmad_loop/adapters/psmux_backend.py index 4b741867..bf336de2 100644 --- a/src/bmad_loop/adapters/psmux_backend.py +++ b/src/bmad_loop/adapters/psmux_backend.py @@ -555,6 +555,17 @@ def list_window_ids(self, session: str) -> list[str]: # psmux's list-windows emits bare `@N` lines; qualify them identically # to new_window or window_alive's membership check (native_id in # list_window_ids) would read every window as dead. + # + # No `_SESSION_GONE_STDERR` override, and that is a measurement rather + # than an omission (#525). psmux 3.3.8 words a vanished session as + # `psmux: no server running on session ''` — the base's fragment + # matches it — and its client-side variant carries `can't find session` + # too. The failures that must NOT read as gone are worded well clear of + # both: a live session whose key was rejected answers `psmux: Invalid + # session key`, and one whose server is unreachable answers `psmux: + # connection timed out`. Both were rc 1 with the windows demonstrably + # alive — this backend is where the bug was actually reachable, because + # a per-session TCP server has failure modes tmux's socket does not. return [ self._qualified_window_id(session, window_id) for window_id in super().list_window_ids(session) @@ -594,6 +605,16 @@ def list_windows(self, session: str, fields: list[str]) -> list[tuple[str, ...]] id_columns = {i for i, field in enumerate(fields) if field == "window_id"} if not opt_columns and not id_columns: return rows + if not rows: + # No rows to fill, so the option listing below has nothing to fill + # them WITH — a pure short-circuit, byte-identical output. It is + # load-bearing anyway (#525): the base answers [] both for a session + # it proved gone and for a spawn that never landed, silently in each + # case, and pressing on would spend a second probe and then warn that + # the option listing failed — about a session that is legitimately + # gone, or on a box with no multiplexer at all. The base's honest + # silences must not be re-broken by the wrapper that reads them. + return rows # The #221 degrade: an empty or `:`-bearing session cannot be routed # with `-t`, and an unrouted read would answer from whichever server # the fallback picks — fill "" without issuing reads at all. @@ -901,7 +922,12 @@ def _scoped_options(self, session: str) -> dict[str, str] | None: surprising {} is possible and is not proof that no keys are set.""" try: proc = self._run(["show-options", "-q", "-t", session], check=False) - except (subprocess.SubprocessError, OSError): + except (subprocess.SubprocessError, OSError, UnicodeError): + # UnicodeError as in the base's listings (#525): this is the SECOND + # probe of a two-probe read, so a strict-codec leaf that decoded the + # window listing cleanly can still fault here — and the caller above + # is a best-effort metadata op that must degrade to "unset", never + # raise a decode error out of it. return None if proc.returncode != 0: return None @@ -951,10 +977,12 @@ def _sweep_orphan_keys(self, session: str) -> None: # A session being swept just minted a window, so an empty live # list is a failed probe, not an empty session — treating it as # truth would sweep every key, live windows included. Warned for - # the same reason the listing failure above is, and this is the - # branch that actually fires: list_window_ids RAISES on a - # transport fault (caught below) and answers [] only on rc != 0, - # so silence here is a server failing every launch with no signal. + # the same reason the listing failure above is. Since #525 the + # only way to reach here is a listing that PROVED the session + # gone — every other failure raises and lands in the arm below — + # which under a just-minted window means the server died between + # the mint and the sweep. Its keys died with it, so the warning + # is the whole remaining duty. print( f"warning: orphan-key sweep on {session} could not list live " "windows; orphaned keys unswept until the next launch", @@ -978,10 +1006,11 @@ def kill_window(self, target: str) -> None: # (the project tag scopes the prune retry; the return key keeps both # return legs armed, see _parked_trailer). Scope resolves before the kill because a name # token cannot be resolved once the window is dead. An empty liveness - # listing is ambiguous — a failed probe, or a session that died with - # its last window — so it degrades toward retaining the keys; the - # launch-time orphan sweep reclaims them once the window is provably - # gone. Discovery is generic by the seam's marker — the backend must + # listing means the session is gone (#525 narrowed it to that; anything + # unproven raises into the arm below), and its keys went with the + # server — but this path cannot tell that from the pre-#525 reading, so + # it still degrades toward retaining them; the launch-time orphan sweep + # reclaims whatever survived once the window is provably gone. Discovery is generic by the seam's marker — the backend must # not know which option names callers use. Best-effort throughout: # cleanup failure warns (the sweep precedent) but never blocks or # fails the kill. diff --git a/src/bmad_loop/adapters/tmux_base.py b/src/bmad_loop/adapters/tmux_base.py index c52500f0..7ce6e7d0 100644 --- a/src/bmad_loop/adapters/tmux_base.py +++ b/src/bmad_loop/adapters/tmux_base.py @@ -6,7 +6,9 @@ the native-Windows :mod:`.psmux_backend` leaf — can subclass :class:`BaseTmuxBackend` and swap only class attributes (:attr:`BaseTmuxBackend._BINARY` for the spawned binary, :attr:`BaseTmuxBackend._ENCODING` / :attr:`BaseTmuxBackend._ERRORS` -for output decoding — a scrubbed +for output decoding, :attr:`BaseTmuxBackend._SESSION_GONE_STDERR` for the stderr +wordings that prove a session is gone rather than a listing merely failed, #525 +— a scrubbed per-call ``env`` is a ``_run`` parameter, and an :meth:`BaseTmuxBackend._run` override is left for timeout tweaks) plus the shell-dialect hooks (``_shell_wrap``, ``_join_argv``, ``_parked_trailer``, ``_source_prefix``, ``_window_launch`` and the @@ -65,6 +67,14 @@ class BaseTmuxBackend(TerminalMultiplexer): #: mid-capture. Honored even where :attr:`_ENCODING` is None, so POSIX keeps #: the locale codec and only stops being strict. A leaf may still override. _ERRORS: str | None = "backslashreplace" + #: stderr fragments (matched case-insensitively, as substrings) that PROVE + #: the target session is gone, as opposed to a listing that merely failed. + #: This is the whole discrimination rule behind :meth:`_session_proved_gone` + #: — see it for why the answer has to be stderr and why the tuple is narrow. + #: A tmux-family leaf whose multiplexer words this differently overrides the + #: tuple; psmux deliberately does not (its own wordings are covered — see + #: the note in :mod:`.psmux_backend`). + _SESSION_GONE_STDERR: tuple[str, ...] = ("no server running", "can't find session") #: Diagnostic from the last :meth:`version` probe (see #: :meth:`TerminalMultiplexer.version_error`). A class-level default so an #: instance that never probed answers None instead of AttributeError. @@ -109,6 +119,99 @@ def _run( raise TmuxError(f"{self._BINARY} {' '.join(argv[:2])} failed: {proc.stderr.strip()}") return proc + def _session_proved_gone(self, proc: subprocess.CompletedProcess[str]) -> bool: + """Whether a non-zero listing exit PROVES the session no longer exists. + + The discrimination the seam's ``[]`` rests on (#525). A non-zero exit + covers two unrelated answers: the session is gone (an ordinary, + knowable fact) and the listing could not be taken at all (a server that + errored while its windows are alive, a rejected auth, an unreachable + port). Folding both to ``[]`` reports the second as "this session has + no windows" — a death the engine acts on and a kill the prune reports + as verified. + + Only stderr can tell them apart: the exit CODE cannot (both multiplexers + exit 1 for everything), and a confirming ``has_session`` round trip + cannot either — that predicate folds every non-zero exit to False by its + own documented contract, so it answers "gone" for exactly the auth and + timeout faults this discrimination exists to catch. + + The tuple is an ALLOWLIST, deliberately: an unrecognized wording is + unknowable, never proof. The failure mode of a too-narrow tuple (a + vanished session read as unverifiable) is loud and self-clearing; a + too-wide one silently restores the bug. Measured rather than assumed, + on tmux 3.4 and psmux 3.3.8 — see the ``_SESSION_GONE_STDERR`` rows in + tests/test_multiplexer.py, which carry the transcript. Neither binary + localizes these strings, so a case-insensitive substring match is enough. + + Both operands are folded, not just the captured text: the case-folding is + a promise to the OVERRIDING leaf, and folding one side only would honor it + for the base's own (already lower-case) tuple while silently breaking a + leaf that spelled its fragment the way its binary prints it. A BLANK + fragment is dropped rather than matched: ``"" in err`` is true for every + error there is, and ``" " in err`` for very nearly as many — any message + with a space in it. Either is #525 restored in full and in silence, from + an authoring slip (a trailing comma, a fragment built from config) that + no reviewer would see, so the two slips that could reintroduce the bug + cannot. The fragment is dropped, not stripped: a leaf that meant a + leading space meant it, and silently re-anchoring its fragment would + substitute a different rule for the one it declared. + """ + err = proc.stderr.lower() + return any(f.lower() in err for f in self._SESSION_GONE_STDERR if f.strip()) + + def _warn_unproven_listing( + self, verb: str, proc: subprocess.CompletedProcess[str] | BaseException + ) -> None: + """Say out loud that a METADATA listing failed for a reason other than the + session being gone (#525). + + The same lens as :meth:`list_window_ids`, the opposite conclusion: these + callers read tags and window rows, not liveness, and their sentinel + degrades toward doing NOTHING — an empty candidate list prunes nothing, + an unread tag reads as untagged and is left alone — so the documented + ``[]`` / ``{}`` stays. What was missing is the signal: a server erroring + on every call made the tool behave as if the sessions it manages had + simply stopped existing, with nothing on stderr to say why. Silent for a + genuinely gone session, which is an answer, not a fault. + + Takes the completed process OR the transport exception that replaced one: + a timeout and a spawn that died prove no more about the session than an + unrecognized non-zero exit does, and the caller's sentinel is the same, + so the signal must be too. + + The one failure left deliberately silent is a multiplexer that is not + installed at all: a box without one has no sessions to report on, so the + absence is an answer rather than a fault (the standing reading, see + :meth:`list_sessions`). That check lives HERE, gating only the + diagnostic, and must not be hoisted into the callers as an early return. + A ``shutil.which`` short-circuit ahead of :meth:`_run` decides the + RETURN VALUE from the ambient PATH, which makes the seam unreachable + through the one spawn primitive — every caller and every test that + injects a transport gets the sentinel no matter what it injected, and + the divergence hides on whichever platform happens to have the binary. + Gating the warning costs one failed exec on a binary-less box and keeps + the answer where the contract says it comes from. + + Warn-only by construction, like every other diagnostic in this module: + under the TUI stderr is captured for the app's whole run (see + ``tui/app.py``), so this is a CLI-visible signal. + """ + if not shutil.which(self._BINARY): + return + if isinstance(proc, BaseException): + outcome, detail = "failed", f"{type(proc).__name__}: {proc}" + else: + if self._session_proved_gone(proc): + return + outcome = f"exited {proc.returncode}" + detail = proc.stderr.strip() or "(no stderr)" + print( + f"warning: {self._BINARY} {verb} {outcome} without proving " + f"the session gone; reading it as empty: {detail}", + file=sys.stderr, + ) + def _tmux(self, *args: str) -> str: # The strict form: a non-zero exit already raises TmuxError inside _run. # A timeout / missing binary escapes _run raw, so trap it here once and @@ -190,9 +293,15 @@ def session_options(self, option: str) -> dict[str, str]: proc = self._run( ["list-sessions", "-F", f"#{{session_name}}\t#{{{option}}}"], check=False ) - except (subprocess.SubprocessError, OSError): + except (subprocess.SubprocessError, OSError, UnicodeError) as exc: + # UnicodeError for the reason list_window_ids names it: a leaf that + # overrides _ERRORS back to a strict handler raises a ValueError-family + # decode error neither other arm covers, and this method's contract is + # warn-and-sentinel, never a raise. + self._warn_unproven_listing("list-sessions", exc) return {} if proc.returncode != 0: # no server / no sessions + self._warn_unproven_listing("list-sessions", proc) return {} options: dict[str, str] = {} for line in proc.stdout.splitlines(): @@ -318,11 +427,15 @@ def list_window_ids(self, session: str) -> list[str]: # display-message -t exits 0 with empty output, so list the # session's window ids and check membership instead. # - # A transport failure (timeout / missing binary) must RAISE, not return []. - # window_alive() is the engine's liveness probe; a sentinel [] would falsely - # read as "window dead -> session crashed" on a mere tmux hang. The honest - # answer to "is it alive?" is "unknowable" -> MultiplexerError. A real dead - # window still returns [] via the returncode != 0 path below (no exception). + # A failed listing must RAISE, not return []. window_alive() is the engine's + # liveness probe; a sentinel [] would falsely read as "window dead -> session + # crashed". The honest answer to "is it alive?" is "unknowable" -> + # MultiplexerError. That covers the transport failures below (timeout / + # missing binary / decode) AND a non-zero exit that does not prove the + # session is gone (#525) — see _session_proved_gone for the discrimination + # and why an exit code alone cannot make it. A session that really vanished + # still returns [] (no exception): the prune must be able to report its + # kills as removed rather than invent a phantom survivor. # # UnicodeError is a transport failure too. _run no longer decodes strictly # on any platform (_ERRORS is backslashreplace, #380), so this arm is now @@ -339,7 +452,12 @@ def list_window_ids(self, session: str) -> list[str]: except (subprocess.TimeoutExpired, OSError, UnicodeError) as exc: raise TmuxError(f"{self._BINARY} list-windows failed: {exc}") from exc if probe.returncode != 0: - return [] + if self._session_proved_gone(probe): + return [] + raise TmuxError( + f"{self._BINARY} list-windows on {session} exited {probe.returncode} " + f"without proving the session gone: {probe.stderr.strip() or '(no stderr)'}" + ) return probe.stdout.split() def pipe_pane(self, window_id: str, log_file: Path) -> None: @@ -447,12 +565,26 @@ def window_pane_pids(self, target: str) -> list[int]: return [] def list_windows(self, session: str, fields: list[str]) -> list[tuple[str, ...]]: + # No missing-binary pre-gate here, deliberately, unlike list_sessions / + # session_options: those two answer "what exists" and may decide that from + # the ambient PATH, but this one is reached with a session already in hand + # and its answer must come from _run — the one spawn primitive the seam + # documents. A `shutil.which` short-circuit ahead of it returns the + # sentinel without consulting the transport at all, so an injected _run is + # never asked, and the behavior silently diverges by whether the binary + # happens to be on THIS box's PATH. The no-multiplexer silence this was + # reaching for is gated inside _warn_unproven_listing instead, where it + # costs a failed exec and nothing else. fmt = "\t".join(f"#{{{field}}}" for field in fields) try: probe = self._run(["list-windows", "-t", f"={session}", "-F", fmt], check=False) - except (subprocess.SubprocessError, OSError): + except (subprocess.SubprocessError, OSError, UnicodeError) as exc: + # UnicodeError as in session_options: a strict-codec leaf must get the + # documented sentinel, not a raw decode error out of a best-effort op. + self._warn_unproven_listing("list-windows", exc) return [] if probe.returncode != 0: + self._warn_unproven_listing("list-windows", probe) return [] rows: list[tuple[str, ...]] = [] for line in probe.stdout.splitlines(): diff --git a/src/bmad_loop/tui/launch.py b/src/bmad_loop/tui/launch.py index b9ae3efc..a1bcdfb9 100644 --- a/src/bmad_loop/tui/launch.py +++ b/src/bmad_loop/tui/launch.py @@ -749,19 +749,19 @@ def prune_ctl_windows(project: Path) -> tuple[list[str], list[str], list[str]]: set membership either way, so the verdict costs one extra round trip instead of N. A transport fault raises and nothing can be claimed there. - Two ceilings, both deliberate: - - - The membership test pairs list_windows' `window_id` column with - list_window_ids. The seam states its symmetry rules pairwise and this pair - is stated because of THIS caller — a backend qualifying one side and not - the other reads every candidate as removed, which is #435 restored on the - optimistic side, with no error anywhere. - - `[]` is read as "the session went with its last window". The seam's `[]` - is wider than that: BaseTmuxBackend folds EVERY nonzero exit to `[]`, so a - server that errors while its windows live would report them removed. The - common cause by far is the session really being gone, and pessimism there - would invent a phantom survivor on every future sweep. Narrowing the - sentinel is a change to the engine's liveness probe, not to this function. + One ceiling, deliberate: the membership test pairs list_windows' `window_id` + column with list_window_ids. The seam states its symmetry rules pairwise and + this pair is stated because of THIS caller — a backend qualifying one side + and not the other reads every candidate as removed, which is #435 restored + on the optimistic side, with no error anywhere. + + `[]` is read as "the session went with its last window", and since #525 the + seam means exactly that: a listing that merely FAILED raises instead of + folding to `[]`, so it lands in `unverifiable` below rather than reporting + every candidate removed. The narrow reading is what keeps the pessimism + honest in the other direction too — a genuinely vanished session still + answers `[]`, so its kills are reported as removed instead of leaving a + phantom survivor for every future sweep to re-report. """ mux = get_multiplexer() candidates = _ctl_window_candidates(project) diff --git a/tests/test_multiplexer.py b/tests/test_multiplexer.py index 500cc71b..81a8ee94 100644 --- a/tests/test_multiplexer.py +++ b/tests/test_multiplexer.py @@ -321,6 +321,265 @@ def test_seam_methods_never_leak_raw_subprocess_error(boom_run, tmp_path): assert mux.current_pane_id() is None +# --------------------------------------------- proved-gone vs unknowable (#525) +# +# The seam's `[]` is a positive claim: the backend listed the session and found +# no windows, OR established that the session is gone. Every OTHER non-zero exit +# is a listing that could not be taken, and answering `[]` there reports a live +# session as empty — a death the engine acts on, a kill the prune calls verified. +# +# The rows below are a TRANSCRIPT, not invented fixtures. Measured 2026-08-31 +# against the real binaries, tmux 3.4 (WSL) and psmux 3.3.8 (66cf613): +# +# tmux list-windows -t =ghost rc 1 "can't find session: ghost" +# tmux list-windows, no server rc 1 "no server running on /tmp/tmux-1000/default" +# psmux list-windows -t =ghost rc 1 "psmux: no server running on session 'ghost'" +# psmux, tampered .key, WINDOWS ALIVE rc 1 "psmux: Invalid session key" +# psmux, wrong .port, WINDOWS ALIVE rc 1 "psmux: connection timed out" +# +# The last two are why an exit code cannot make this call: identical rc, live +# windows. A rejected `-F` format is NOT on the list because neither binary +# fails on one — both exit 0 (tmux prints nothing, psmux echoes the literal), so +# there is no "rejected format" arm to discriminate. + +_PROVED_GONE_STDERR = [ + "can't find session: ghost", + "no server running on /tmp/tmux-1000/default", + "psmux: no server running on session 'ghost'", +] +_UNPROVEN_STDERR = [ + "psmux: Invalid session key", + "psmux: connection timed out", + "psmux: no response from server (timed out)", + "", # a non-zero exit that said nothing at all proves nothing at all +] + + +def _failing_listing(monkeypatch, stderr: str) -> None: + monkeypatch.setattr(tmux_base.shutil, "which", lambda _name: "/usr/bin/tmux") + monkeypatch.setattr( + tmux_base.subprocess, + "run", + lambda argv, **_k: subprocess.CompletedProcess(argv, 1, stdout="", stderr=stderr), + ) + + +@pytest.mark.parametrize("stderr", _PROVED_GONE_STDERR) +def test_list_window_ids_answers_empty_when_the_session_is_proved_gone(monkeypatch, stderr): + """A vanished session is an ANSWER, not a fault: `[]`, no exception. + + Raising here instead would be the same dishonest report from the other + side — prune_ctl_windows would invent a phantom survivor that every + subsequent cleanup re-reports and nothing ever clears.""" + _failing_listing(monkeypatch, stderr) + assert TmuxMultiplexer().list_window_ids("s") == [] + assert TmuxMultiplexer().window_alive("s", "@1") is False + + +@pytest.mark.parametrize("stderr", _UNPROVEN_STDERR) +def test_list_window_ids_raises_when_a_nonzero_exit_proves_nothing(monkeypatch, stderr): + """The bug (#525): a listing that failed while the windows are alive must not + read as an empty session. + + Both live-window rows come from a psmux server that was still serving its + windows — one refused the auth, one was unreachable — and folding either to + `[]` tells the engine's liveness probe the window died and the prune that + its kills are verified. + + Ablation: replace the `_session_proved_gone` guard with a bare `return []` + and every row here fails, while the proved-gone sibling above still passes. + """ + _failing_listing(monkeypatch, stderr) + with pytest.raises(MultiplexerError): + TmuxMultiplexer().list_window_ids("s") + with pytest.raises(MultiplexerError): + TmuxMultiplexer().window_alive("s", "@1") + + +def test_gone_stderr_match_is_case_insensitive_and_substring(monkeypatch): + """The fragments are matched inside a longer line and without regard to case: + every measured wording embeds them in a sentence (`psmux: no server running + on session 'x'`), and a backend that capitalizes its first word must not + thereby turn a vanished session into an unverifiable one.""" + _failing_listing(monkeypatch, "PSMUX: No Server Running on session 'ghost'\n") + assert TmuxMultiplexer().list_window_ids("s") == [] + + +def test_a_leaf_may_override_the_proved_gone_wordings(monkeypatch): + """The rule is a class attribute so a tmux-family leaf whose multiplexer words + absence differently can replace it without touching a method body — the same + swap-a-class-attr contract as `_BINARY` / `_ENCODING`. + + psmux deliberately does NOT override (its measured wordings are covered by + the base tuple); this locks the seam open for the leaf that isn't.""" + + class Dialect(TmuxMultiplexer): + # Mixed case ON THE FRAGMENT, deliberately: the case-folding is a promise + # to the leaf, and folding only the captured text keeps the base's own + # (lower-case) tuple working while silently breaking every override that + # spelled its fragment the way its binary prints it — a vanished session + # read as unverifiable, i.e. a phantom survivor forever. + _SESSION_GONE_STDERR = ("Session Vanished",) + + _failing_listing(monkeypatch, "the session vanished, sorry") + assert Dialect().list_window_ids("s") == [] + # ...and the base's own wordings stop counting once replaced + _failing_listing(monkeypatch, "can't find session: ghost") + with pytest.raises(MultiplexerError): + Dialect().list_window_ids("s") + + +@pytest.mark.parametrize("blank", ["", " ", "\t", "\n"]) +def test_a_blank_gone_fragment_never_matches(monkeypatch, blank): + """A blank fragment in the tuple must be dropped, not matched. + + `"" in err` is true for every error there is, and `" " in err` for very + nearly as many — any message with a space in it, which is all of them. So + one stray element (a trailing comma, a leaf building its tuple from config) + folds EVERY failed listing back to `[]`: #525 restored in full, in the + silent direction, from a slip no reviewer would see. + + The whitespace rows are the point — an emptiness check that only rejects + `""` leaves the more likely slip live. Ablation: weaken `if f.strip()` to + `if f` and the space and newline rows fail. The tab row passes either way, + because no message these binaries emit contains one; it is here to pin the + rule, not to drive the ablation. The stderr below keeps its trailing newline + for the same reason the wordings are verbatim — that is how a real binary + hands it over, and it is what makes the `"\\n"` row a live threat rather than + a hypothetical one.""" + + class Sloppy(TmuxMultiplexer): + _SESSION_GONE_STDERR = ("no server running", blank) + + _failing_listing(monkeypatch, "psmux: Invalid session key\n") + with pytest.raises(MultiplexerError): + Sloppy().list_window_ids("s") + # the real sibling still counts — the filter drops the element, not the rule + _failing_listing(monkeypatch, "no server running on /tmp/tmux-1000/default") + assert Sloppy().list_window_ids("s") == [] + + +@pytest.mark.parametrize("stderr", _UNPROVEN_STDERR) +def test_metadata_listings_keep_their_sentinel_but_say_so(monkeypatch, capsys, stderr): + """Same lens, opposite conclusion (#525). `list_windows` and `session_options` + read metadata, not liveness, and their sentinel degrades toward doing + NOTHING — an empty candidate list prunes nothing, an unread tag reads as + untagged and is left alone — so the documented `[]` / `{}` stays. + + What was missing is the signal: a server erroring on every call made the + tool behave as if the sessions it manages had stopped existing, silently.""" + _failing_listing(monkeypatch, stderr) + mux = TmuxMultiplexer() + assert mux.list_windows("s", ["window_id"]) == [] + assert mux.session_options("@opt") == {} + err = capsys.readouterr().err + assert err.count("without proving the session gone") == 2 + # A non-zero exit that said NOTHING proves nothing either, and the warning has + # to survive having no detail to quote — the row this parametrization used to + # drop, under which a silent-on-blank-stderr regression passed. + assert (stderr.strip() or "(no stderr)") in err + + +def test_metadata_listings_warn_when_the_transport_itself_failed(boom_run, capsys): + """A timeout / a spawn that died proves no more about the session than an + unrecognized non-zero exit, and the sentinel is identical — so the signal has + to be too, or the seam's "say when the failure proved nothing" is only half + true and the quietest failures are the ones that stay quiet. + + The `shutil.which` pre-gate is the deliberate exception and is covered by + test_seam_methods_never_leak_raw_subprocess_error's no-binary case: a box + with no multiplexer has no sessions to report on.""" + mux = TmuxMultiplexer() + assert mux.list_windows("s", ["window_id"]) == [] + assert mux.session_options("@opt") == {} + err = capsys.readouterr().err + assert err.count("without proving the session gone") == 2 + assert type(boom_run).__name__ in err + + +def test_metadata_listings_contain_a_decode_fault(monkeypatch, capsys): + """A strict-codec leaf must get the documented sentinel here too, not a raw + UnicodeDecodeError out of a method the seam calls best-effort. + + `list_window_ids` has named this arm since #380 — a leaf overriding `_ERRORS` + back to a strict handler raises a ValueError-family decode error that neither + `SubprocessError` nor `OSError` covers. The metadata siblings promise `[]` / + `{}` on a transport failure and a decode fault is one, so the same arm belongs + here; without it the promise holds for every failure except this one.""" + monkeypatch.setattr(tmux_base.shutil, "which", lambda _name: "/usr/bin/tmux") + + def boom(*_a, **_k): + raise UnicodeDecodeError("utf-8", b"\xff", 0, 1, "invalid start byte") + + monkeypatch.setattr(tmux_base.subprocess, "run", boom) + mux = TmuxMultiplexer() + assert mux.list_windows("s", ["window_id"]) == [] + assert mux.session_options("@opt") == {} + assert capsys.readouterr().err.count("without proving the session gone") == 2 + + +def test_metadata_listings_are_silent_without_a_binary(monkeypatch, capsys): + """No multiplexer installed is an ANSWER, not a fault: both metadata listings + answer their sentinel and say nothing. + + Uniformity is the assertion. A sibling that warned here would fire on every + call on such a box while the other stayed quiet, and a diagnostic that fires + for one method and not its twin teaches the reader to ignore it. + + Silent, NOT spawnless, and the difference is the whole point. The silence is + gated inside `_warn_unproven_listing`, after `_run` has been asked; gating it + by short-circuiting ahead of `_run` would decide the RETURN VALUE from the + ambient PATH, which makes the seam unreachable through its own spawn + primitive — the injected transport below would never be consulted, and the + resulting behavior would differ by platform rather than by contract. That + was a real regression: it left `list_windows` answering `[]` for every + psmux test on Linux, where the binary does not exist, while staying green on + Windows, where it does. + + Ablation: drop the `shutil.which` guard in `_warn_unproven_listing` and this + fails on the two warnings it must not print.""" + monkeypatch.setattr(tmux_base.shutil, "which", lambda _name: None) + monkeypatch.setattr( + tmux_base.subprocess, + "run", + lambda argv, **_k: (_ for _ in ()).throw(FileNotFoundError(argv[0])), + ) + mux = TmuxMultiplexer() + assert mux.list_windows("s", ["window_id"]) == [] + assert mux.session_options("@opt") == {} + assert capsys.readouterr().err == "" + + +def test_list_windows_answer_comes_from_run_not_from_path(monkeypatch): + """`list_windows` must consult `_run` even when the binary is absent from PATH. + + The seam documents `_run` as the ONE place a spawn happens and the source of + every answer. A `shutil.which` short-circuit ahead of it silently substitutes + the ambient PATH for the transport, so a caller (or a test) that injects a + transport is never asked — which is exactly how the same suite passed on + Windows and failed 15 ways on Linux. + + Ablation: reinstate an early `if not shutil.which(...): return []` in + `list_windows` and this fails on the injected rows never arriving.""" + monkeypatch.setattr(tmux_base.shutil, "which", lambda _name: None) + monkeypatch.setattr( + tmux_base.subprocess, + "run", + lambda argv, **_k: subprocess.CompletedProcess(argv, 0, stdout="@1\tshell\n", stderr=""), + ) + assert TmuxMultiplexer().list_windows("s", ["window_id", "window_name"]) == [("@1", "shell")] + + +def test_metadata_listings_stay_silent_for_a_gone_session(monkeypatch, capsys): + """A vanished session is an answer, not a fault — warning about it would put + noise on the ordinary path (`cleanup` after the last session ends).""" + _failing_listing(monkeypatch, "no server running on /tmp/tmux-1000/default") + mux = TmuxMultiplexer() + assert mux.list_windows("s", ["window_id"]) == [] + assert mux.session_options("@opt") == {} + assert capsys.readouterr().err == "" + + def test_list_window_ids_decode_fault_raises_the_seam_type(monkeypatch): """A byte the codec cannot decode is a transport failure like a timeout: the liveness probe must answer MultiplexerError ("unknowable"), not leak the raw diff --git a/tests/test_psmux_backend.py b/tests/test_psmux_backend.py index a9b17f9c..4008d493 100644 --- a/tests/test_psmux_backend.py +++ b/tests/test_psmux_backend.py @@ -203,6 +203,81 @@ def test_list_windows_id_column_is_findable_in_list_window_ids(monkeypatch): assert all(row[0] in live for row in rows) +def test_list_windows_does_not_probe_options_for_an_empty_listing(monkeypatch, capsys): + """An empty window listing ends the read here — no option probe, no warning. + + The base answers `[]` for a missing binary — after one failed spawn, silently — + and for a session it PROVED gone (#525). Both are honest silences, and this + wrapper is what would re-break them: it fetches the id-keyed options + whenever an `@` column is asked for, so pressing on past an empty listing + spends a second probe and then warns that the option listing failed — on a + box with no multiplexer at all, or about a session that is legitimately + gone. Since there are no rows to fill, the fetch could only ever be waste. + + Ablation: drop the `if not rows` short-circuit and both halves fail — the + gone case gains a show-options spawn plus a warning, the unspawnable case + gains a second doomed spawn and the same stray warning.""" + spawned: list[list[str]] = [] + + def gone(argv, **_kwargs): + spawned.append(argv) + return subprocess.CompletedProcess( + argv, 1, stdout="", stderr="psmux: no server running on session 's'" + ) + + monkeypatch.setattr(tmux_base.shutil, "which", lambda _name: "C:/bin/psmux.exe") + monkeypatch.setattr(tmux_base.subprocess, "run", gone) + mux = PsmuxMultiplexer() + assert mux.list_windows("s", ["window_id", "@bmad_project"]) == [] + assert [a[1] for a in spawned] == ["list-windows"] # never show-options + assert capsys.readouterr().err == "" + + # ...and when the binary cannot be spawned at all, one doomed attempt, not + # two: the base returns [] from the failed spawn and this wrapper stops + # there rather than sending a second one after options that cannot exist. + # The transport says so, never PATH — `which` decides only whether the + # failure is worth a warning, so this half holds identically on a box that + # has psmux and one that does not. + spawned.clear() + monkeypatch.setattr(tmux_base.shutil, "which", lambda _name: None) + monkeypatch.setattr( + tmux_base.subprocess, + "run", + lambda argv, **_k: spawned.append(argv) or _raise(FileNotFoundError(argv[0])), + ) + assert mux.list_windows("s", ["window_id", "@bmad_project"]) == [] + assert [a[1] for a in spawned] == ["list-windows"] + assert capsys.readouterr().err == "" + + +def _raise(exc: BaseException): + raise exc + + +def test_scoped_options_contains_a_decode_fault(monkeypatch, capsys): + """The SECOND probe of the two-probe read must degrade, not raise. + + A strict-codec leaf can decode the window listing cleanly and still fault on + the option listing, so `_scoped_options` needs the same `UnicodeError` arm + the base's listings carry (#525). Without it a raw `UnicodeDecodeError` + escapes `list_windows` — a method the seam calls best-effort, whose callers + catch nothing. + + Ablation: drop `UnicodeError` from the catch tuple and this fails on the + raw decode error escaping.""" + + def fake(argv, **_kwargs): + if argv[1] == "show-options": + raise UnicodeDecodeError("utf-8", b"\xff", 0, 1, "invalid start byte") + return subprocess.CompletedProcess(argv, 0, stdout="@1\n", stderr="") + + monkeypatch.setattr(tmux_base.shutil, "which", lambda _name: "C:/bin/psmux.exe") + monkeypatch.setattr(tmux_base.subprocess, "run", fake) + rows = PsmuxMultiplexer().list_windows("s", ["window_id", "@bmad_project"]) + assert rows == [("s:@1", "")] # the option column degrades to "unset" + assert "show-options listing failed" in capsys.readouterr().err + + def test_qualification_degrades_to_bare_on_colon_session(monkeypatch, tmp_path): # A `:` in the session name would split the target at the wrong colon on # replay — both methods degrade to the bare id identically (the #221 rule). @@ -1597,16 +1672,23 @@ def fake(argv, **kwargs): def test_sweep_treats_empty_live_list_as_failed_probe(monkeypatch, tmp_path, capsys): # We just minted a window in this session, so an empty live listing is a # failed probe, not an empty session — believing it would sweep every key, - # live windows included. It is also said out loud: list_window_ids raises on - # a transport fault and answers [] only on rc != 0, so this branch is a - # server failing every launch, and silence would leak keys with no signal. + # live windows included. It is also said out loud: since #525 list_window_ids + # answers [] ONLY for a listing that proved the session gone, so reaching this + # branch right after a mint means the server died under us — and silence would + # leak keys with no signal. + # + # The stderr below is load-bearing, not decoration: it is psmux 3.3.8's real + # vanished-session wording. Any other wording now RAISES (the sweep's outer + # arm warns instead), which is a different branch than this test pins. listing = '@bmad_project__blw@2 "live"\n@bmad_project__blw@7 "gone"\n' calls = [] def fake(argv, **kwargs): calls.append(argv) if argv[1] == "list-windows": - return subprocess.CompletedProcess(argv, 1, stdout="", stderr="no server") + return subprocess.CompletedProcess( + argv, 1, stdout="", stderr="psmux: no server running on session 'ctl'" + ) out = {"new-window": "@2\n", "show-options": listing}.get(argv[1], "") return subprocess.CompletedProcess(argv, 0, stdout=out, stderr="") diff --git a/tests/test_tui_launch.py b/tests/test_tui_launch.py index 920a4adc..bba68aa7 100644 --- a/tests/test_tui_launch.py +++ b/tests/test_tui_launch.py @@ -1387,12 +1387,26 @@ def fake(argv, **kwargs): raise OSError("server gone") if kill == "undecodable": raise UnicodeDecodeError("utf-8", b"\xff", 0, 1, "invalid start byte") + if kill == "unproven-nonzero": + # A server that errored while its windows are alive: rc 1, + # and a stderr that proves nothing about the session. psmux + # 3.3.8's verbatim answer to a listing whose auth the live + # server rejected — the shape #525 is about. + return subprocess.CompletedProcess( + argv, 1, stdout="", stderr="psmux: Invalid session key" + ) if kill == "session-gone": # rc 1, not rc 0 with empty stdout: real tmux answers a # vanished session with a nonzero exit and list_window_ids # folds it to [] — same verdict, and the path the transport - # actually takes. - return subprocess.CompletedProcess(argv, 1, stdout="", stderr="") + # actually takes. The stderr is tmux 3.4's verbatim wording + # and is load-bearing since #525: it is what makes this a + # PROVED vanish rather than a listing that merely failed, + # and an rc 1 without it now lands in `unverifiable` + # instead (test_prune_ctl_windows_..._unproven_nonzero). + return subprocess.CompletedProcess( + argv, 1, stdout="", stderr=f"can't find session: {launch.CTL_SESSION}" + ) gone = {a[-1] for a in killed} if kill == "lands" else set() return subprocess.CompletedProcess( argv, 0, stdout="\n".join(r[0] for r in rows if r[0] not in gone), stderr="" @@ -1490,9 +1504,15 @@ def test_prune_ctl_windows_undecodable_liveness_is_a_transport_fault(monkeypatch def test_prune_ctl_windows_reads_an_empty_listing_as_the_session_going_with_it( monkeypatch, tmp_path: Path ): - """`[]` is the seam's "no windows", not a failed probe (only a transport fault - raises) — a ctl session that died with its last window really did take the - candidate, so pessimism here would report a phantom survivor forever.""" + """`[]` is the seam's "no windows", not a failed probe — a ctl session that + died with its last window really did take the candidate, so pessimism here + would report a phantom survivor forever. + + The other half of #525's discrimination, and the reason narrowing the + sentinel could not simply be "raise on rc != 0": a vanished session exits + non-zero too, and turning THAT into `unverifiable` is the same dishonest + report from the other side — one that every subsequent cleanup re-reports + and nothing ever clears.""" _ctl_prune_fake(monkeypatch, tmp_path, kill="session-gone") assert launch.prune_ctl_windows(tmp_path) == ( @@ -1502,6 +1522,30 @@ def test_prune_ctl_windows_reads_an_empty_listing_as_the_session_going_with_it( ) +def test_prune_ctl_windows_unproven_nonzero_listing_claims_nothing(monkeypatch, tmp_path: Path): + """A listing that exits non-zero WITHOUT proving the session gone is a failed + probe, and the kills it was meant to verify stay unverifiable (#525). + + This is the exact over-optimistic report #435 exists to eliminate, reached + by the one route it left open: the backend folded every non-zero exit to + `[]`, so a server erroring while its windows are alive answered "this + session has no windows" and every candidate was classified verifiably + removed — with no error anywhere and nothing to re-try. + + Ablation: drop the `_session_proved_gone` guard in + `BaseTmuxBackend.list_window_ids` (return `[]` on any non-zero exit) and + this fails on `removed` carrying both windows, while its `session-gone` + sibling above still passes — the two together pin the discrimination + rather than either direction alone.""" + _ctl_prune_fake(monkeypatch, tmp_path, kill="unproven-nonzero") + + assert launch.prune_ctl_windows(tmp_path) == ( + [], + [], + ["sweep-20260101-000000-dead", "run-20260101-000000-dead2"], + ) + + def test_prune_ctl_windows_with_no_candidates_never_probes(monkeypatch, tmp_path: Path): """The listing is a real round trip; a prune with nothing to kill must not pay for it (and must not read an empty ctl session as anything at all)."""