Skip to content

Commit 0950630

Browse files
committed
Mcp(fix[pane]): Stop reporting tmux failures as success
Two defects on the same seam: an operation failed and the server told the agent something that was not true. `send_keys` dropped any payload starting with `-` and returned `Keys sent to pane %N`. tmux read the payload as flags and rejected the command; `Pane.send_keys` builds the argv with no `--` separator and discards tmux's result, and the wrapper returned a hardcoded success string. Reachable with ordinary input — `--help` typed into a REPL, a negative number, a pasted diff line. The argv now ends flag parsing with `--` and a failed send raises with tmux's own stderr. All three call sites built argv separately and shared none of it; they now share one builder, which also fixes the timed batch path (it surfaced the error but still failed to deliver). `wait_for_text` crashed with `ValueError: invalid literal for int() with base 10: ''` when its pane died mid-wait. tmux expands every field of a vanished pane to the empty string and three `int()` calls on the poll path took it raw — the same degrade-don't-fail rule the comment three lines above already states for `alternate_on`. Fixing only the `int()` calls would have swapped the crash for a wrong answer: `pane_dead` is itself blanked, so it reads back as `"0"` and cannot report the death. A killed pane would then have failed the pid comparison and been reported as *respawned*. A live pane always has a `pane_pid`, so an empty one is the reliable gone signal, and the wait now ends with `pane %N died`. `_read_history_limit` had the same defect one call over — its guard covered an empty list but not an empty string. There were no tests for `_parse_pane_state` at all, which is why both defects survived. The three history tests that asserted on `Pane.send_keys` calls now observe the argv boundary, which is where every path already goes.
1 parent 517d0da commit 0950630

5 files changed

Lines changed: 263 additions & 69 deletions

File tree

CHANGES

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,39 @@ _Notes on upcoming releases will be added here_
88

99
### What's new
1010

11+
#### A pane that dies mid-wait reports the death, not a parse crash
12+
13+
Killing a pane while `wait_for_text` was waiting on it surfaced
14+
`Unexpected error: ValueError: invalid literal for int() with base 10:
15+
''`. tmux expands every field of a vanished pane to the empty string,
16+
and three `int()` calls on the poll path took it raw.
17+
18+
Fixing only the `int()` calls would have replaced the crash with a wrong
19+
answer: `pane_dead` is one of the blanked fields, so it reads back as
20+
`"0"` and cannot report the death itself. A killed pane would then have
21+
failed the pid comparison instead and been reported as *respawned*. A
22+
live pane always has a `pane_pid`, so an empty one is the reliable
23+
signal that the pane is gone, and the wait now ends with `pane %N died;
24+
cursor/baseline anchor is no longer valid`.
25+
26+
`_read_history_limit` had the same defect one call over — its guard
27+
covered an empty list but not an empty string — and now shares the
28+
helper.
29+
30+
#### `send_keys` no longer drops text that starts with `-`
31+
32+
`send_keys(keys="-X cancel", literal=True)` returned `Keys sent to pane
33+
%N` and sent nothing. tmux parsed the payload as flags and rejected the
34+
command; the wrapper discarded tmux's result and returned a hardcoded
35+
success string. Reachable with ordinary input — `--help` or `-v` typed
36+
into a REPL, a negative number, a pasted diff line.
37+
38+
The `send-keys` argv now ends flag parsing with `--`, and a failed send
39+
raises with tmux's own stderr instead of reporting success. This covers
40+
`send_keys` and both `send_keys_batch` paths, which each built the argv
41+
separately; the timed batch path surfaced the error but still failed to
42+
deliver.
43+
1144
#### A gated tool now says which tier it needs
1245

1346
Calling a tool above the server's `LIBTMUX_SAFETY` tier reported

src/libtmux_mcp/tools/pane_tools/io.py

Lines changed: 97 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,89 @@ def _remaining_timeout(deadline: float, timeout: float) -> float:
5050
return remaining
5151

5252

53+
#: Bound on a single untimed ``send-keys``. libtmux runs tmux through
54+
#: ``Popen.communicate()`` with no timeout, so an unresponsive server
55+
#: would wedge the tool call. Mirrors ``wait.py``'s per-call ceiling.
56+
_SEND_KEYS_TIMEOUT_SECONDS = 5.0
57+
58+
59+
def _send_keys_argvs(
60+
pane: Pane,
61+
keys: str,
62+
*,
63+
enter: bool,
64+
literal: bool,
65+
suppress_history: bool,
66+
) -> list[list[str]]:
67+
"""Build the ``tmux send-keys`` argv(s) for one send.
68+
69+
``--`` terminates flag parsing. Without it tmux reads a payload
70+
beginning with ``-`` as flags and rejects the command, so `--help`,
71+
a negative number, or a pasted diff line never reaches the pane.
72+
``Pane.send_keys`` omits it and discards tmux's result, which is why
73+
that failure arrived as a success.
74+
75+
Enter is a separate call without ``-l`` so it stays a key name
76+
rather than the literal text ``Enter``.
77+
"""
78+
pane_id = pane.pane_id
79+
if pane_id is None:
80+
msg = "resolved pane has no pane_id"
81+
raise ExpectedToolError(msg)
82+
83+
tmux_args = ["send-keys", "-t", pane_id]
84+
if literal:
85+
tmux_args.append("-l")
86+
tmux_args.extend(("--", (" " if suppress_history else "") + keys))
87+
88+
argvs = [_tmux_argv(pane.server, *tmux_args)]
89+
if enter:
90+
argvs.append(_tmux_argv(pane.server, "send-keys", "-t", pane_id, "Enter"))
91+
return argvs
92+
93+
94+
def _raise_send_keys_error(exc: subprocess.CalledProcessError) -> t.NoReturn:
95+
"""Re-raise a failed ``send-keys`` carrying tmux's own stderr."""
96+
stderr = exc.stderr.decode(errors="replace").strip() if exc.stderr else ""
97+
msg = f"send-keys failed: {stderr or exc}"
98+
raise ExpectedToolError(msg) from exc
99+
100+
101+
def _run_send_keys_argv(argv: list[str]) -> None:
102+
"""Run one ``tmux send-keys`` argv under the untimed ceiling."""
103+
try:
104+
subprocess.run(
105+
argv,
106+
check=True,
107+
capture_output=True,
108+
timeout=_SEND_KEYS_TIMEOUT_SECONDS,
109+
)
110+
except subprocess.TimeoutExpired as e:
111+
msg = f"send-keys timed out after {_SEND_KEYS_TIMEOUT_SECONDS}s"
112+
raise ExpectedToolError(msg) from e
113+
except subprocess.CalledProcessError as e:
114+
_raise_send_keys_error(e)
115+
116+
117+
def _run_send_keys(
118+
pane: Pane,
119+
keys: str,
120+
*,
121+
enter: bool,
122+
literal: bool,
123+
suppress_history: bool,
124+
) -> None:
125+
"""Send keys to *pane*, raising if tmux rejected them."""
126+
for argv in _send_keys_argvs(
127+
pane,
128+
keys,
129+
enter=enter,
130+
literal=literal,
131+
suppress_history=suppress_history,
132+
):
133+
_run_send_keys_argv(argv)
134+
135+
53136
def _run_timed_send_keys_argv(
54137
argv: list[str],
55138
*,
@@ -67,9 +150,7 @@ def _run_timed_send_keys_argv(
67150
except subprocess.TimeoutExpired as e:
68151
raise ExpectedToolError(_batch_timeout_error(timeout)) from e
69152
except subprocess.CalledProcessError as e:
70-
stderr = e.stderr.decode(errors="replace").strip() if e.stderr else ""
71-
msg = f"send-keys failed: {stderr or e}"
72-
raise ExpectedToolError(msg) from e
153+
_raise_send_keys_error(e)
73154

74155

75156
def _run_timed_send_keys(
@@ -80,21 +161,13 @@ def _run_timed_send_keys(
80161
timeout: float,
81162
) -> None:
82163
"""Run ``tmux send-keys`` for one operation within the batch deadline."""
83-
pane_id = pane.pane_id
84-
if pane_id is None:
85-
msg = "resolved pane has no pane_id"
86-
raise ExpectedToolError(msg)
87-
88-
tmux_args = ["send-keys", "-t", pane_id]
89-
if operation.literal:
90-
tmux_args.append("-l")
91-
tmux_args.append((" " if operation.suppress_history else "") + operation.keys)
92-
93-
send_argvs = [_tmux_argv(pane.server, *tmux_args)]
94-
if operation.enter:
95-
send_argvs.append(_tmux_argv(pane.server, "send-keys", "-t", pane_id, "Enter"))
96-
97-
for argv in send_argvs:
164+
for argv in _send_keys_argvs(
165+
pane,
166+
operation.keys,
167+
enter=operation.enter,
168+
literal=operation.literal,
169+
suppress_history=operation.suppress_history,
170+
):
98171
_run_timed_send_keys_argv(argv, deadline=deadline, timeout=timeout)
99172

100173

@@ -161,11 +234,12 @@ def send_keys(
161234
session_id=session_id,
162235
window_id=window_id,
163236
)
164-
pane.send_keys(
237+
_run_send_keys(
238+
pane,
165239
keys,
166240
enter=enter,
167-
suppress_history=suppress_history,
168241
literal=literal,
242+
suppress_history=suppress_history,
169243
)
170244
return f"Keys sent to pane {pane.pane_id}"
171245

@@ -266,11 +340,12 @@ def send_keys_batch(
266340
break
267341
continue
268342
if deadline is None:
269-
pane.send_keys(
343+
_run_send_keys(
344+
pane,
270345
operation.keys,
271346
enter=operation.enter,
272-
suppress_history=operation.suppress_history,
273347
literal=operation.literal,
348+
suppress_history=operation.suppress_history,
274349
)
275350
else:
276351
assert timeout is not None

src/libtmux_mcp/tools/pane_tools/state.py

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,19 @@ class _PaneState(t.NamedTuple):
6262
HISTORY_LIMIT_FORMAT = "#{history_limit}"
6363

6464

65+
def _int_or_zero(value: str) -> int:
66+
"""Parse a tmux numeric format field, treating a missing value as 0.
67+
68+
A dead pane makes ``display-message`` expand every field to the
69+
empty string, so a bare ``int()`` raised
70+
``ValueError: invalid literal for int() with base 10: ''`` from the
71+
hot poll path -- a wait whose pane died reported a raw parse crash
72+
rather than ``pane_dead``. Same degrade-don't-fail rule the
73+
``alternate_on`` comment below states.
74+
"""
75+
return int(value) if value else 0
76+
77+
6578
def _parse_pane_state(raw: str) -> _PaneState:
6679
"""Parse one :data:`PANE_STATE_FORMAT` line into a :class:`_PaneState`."""
6780
# ``maxsplit`` is one below the field count so a pane_pid or a
@@ -73,12 +86,17 @@ def _parse_pane_state(raw: str) -> _PaneState:
7386
parts = raw.split("|", 5)
7487
hs, cy, sy, pid, dead = parts[:5]
7588
alternate = parts[5] if len(parts) > 5 else "0"
89+
# A pane that no longer exists expands EVERY field to empty --
90+
# ``pane_dead`` included, so it reads as "0" and cannot report the
91+
# death itself. A live pane always has a pid, so an empty one is
92+
# the reliable gone signal; without it the pid mismatch below
93+
# reports a killed pane as "respawned".
7694
return _PaneState(
77-
history_size=int(hs),
78-
cursor_y=int(cy),
79-
pane_height=int(sy),
95+
history_size=_int_or_zero(hs),
96+
cursor_y=_int_or_zero(cy),
97+
pane_height=_int_or_zero(sy),
8098
pane_pid=pid,
81-
pane_dead=dead == "1",
99+
pane_dead=dead == "1" or not pid,
82100
alternate_on=alternate == "1",
83101
)
84102

@@ -130,4 +148,4 @@ def _read_history_limit(pane: Pane) -> int:
130148
"""
131149
stdout = pane.display_message(HISTORY_LIMIT_FORMAT, get_text=True)
132150
raw = stdout[0] if stdout else "0"
133-
return int(raw)
151+
return _int_or_zero(raw)

tests/test_history.py

Lines changed: 30 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -666,14 +666,12 @@ def test_global_history_default_leaves_raw_send_keys_bytes_and_boundaries(
666666
) -> None:
667667
"""Control/TUI input stays exact; explicit suppression adds one space.
668668
669-
A fake pane delegates to libtmux's real ``send_keys`` at the pre-PTY
670-
command boundary, where an inherited prefix or merged Enter is observable.
669+
Subprocess interception at the pre-PTY argv boundary, where an
670+
inherited prefix or a merged Enter would be observable.
671671
"""
672-
from libtmux import Pane
673-
674672
from libtmux_mcp.tools.pane_tools import io
675673

676-
calls: list[tuple[str, tuple[str, ...]]] = []
674+
calls: list[list[str]] = []
677675

678676
class FakeServer:
679677
tmux_bin = "tmux"
@@ -684,18 +682,14 @@ class FakePane:
684682
pane_id = "%1"
685683
server = FakeServer()
686684

687-
def cmd(self, *args: str) -> None:
688-
calls.append(("cmd", args))
689-
690-
def enter(self) -> None:
691-
calls.append(("enter", ()))
692-
693-
def send_keys(self, keys: str, **kwargs: t.Any) -> None:
694-
Pane.send_keys(t.cast("Pane", self), keys, **kwargs)
685+
def _run(argv: list[str], **kwargs: t.Any) -> subprocess.CompletedProcess[str]:
686+
calls.append(argv)
687+
return subprocess.CompletedProcess(argv, 0)
695688

696689
pane = FakePane()
697690
monkeypatch.setattr(io, "_get_server", lambda **kwargs: FakeServer())
698691
monkeypatch.setattr(io, "_resolve_pane", lambda *args, **kwargs: pane)
692+
monkeypatch.setattr("libtmux_mcp.tools.pane_tools.io.subprocess.run", _run)
699693

700694
async def _exercise() -> None:
701695
async with Client(_history_server("1")) as client:
@@ -721,12 +715,12 @@ async def _exercise() -> None:
721715
asyncio.run(_exercise())
722716

723717
assert calls == [
724-
("cmd", ("send-keys", "C-c")),
725-
("cmd", ("send-keys", "-l", "partial-TUI")),
726-
("cmd", ("send-keys", "/needle")),
727-
("enter", ()),
728-
("cmd", ("send-keys", "-l", " explicit-secret")),
729-
("enter", ()),
718+
["tmux", "send-keys", "-t", "%1", "--", "C-c"],
719+
["tmux", "send-keys", "-t", "%1", "-l", "--", "partial-TUI"],
720+
["tmux", "send-keys", "-t", "%1", "--", "/needle"],
721+
["tmux", "send-keys", "-t", "%1", "Enter"],
722+
["tmux", "send-keys", "-t", "%1", "-l", "--", " explicit-secret"],
723+
["tmux", "send-keys", "-t", "%1", "Enter"],
730724
]
731725

732726

@@ -735,14 +729,12 @@ def test_global_history_default_leaves_untimed_batch_operations_explicit_only(
735729
) -> None:
736730
"""Untimed batches preserve raw defaults, literal mode, and Enter.
737731
738-
A fake pane exercises libtmux's real ``send_keys`` at the pre-PTY command
739-
boundary so exact literal bytes and the separate Enter call remain visible.
732+
Subprocess interception at the pre-PTY argv boundary keeps the exact
733+
literal bytes and the separate Enter call visible.
740734
"""
741-
from libtmux import Pane
742-
743735
from libtmux_mcp.tools.pane_tools import io
744736

745-
calls: list[tuple[str, tuple[str, ...]]] = []
737+
calls: list[list[str]] = []
746738

747739
class FakeServer:
748740
tmux_bin = "tmux"
@@ -753,18 +745,14 @@ class FakePane:
753745
pane_id = "%1"
754746
server = FakeServer()
755747

756-
def cmd(self, *args: str) -> None:
757-
calls.append(("cmd", args))
758-
759-
def enter(self) -> None:
760-
calls.append(("enter", ()))
761-
762-
def send_keys(self, keys: str, **kwargs: t.Any) -> None:
763-
Pane.send_keys(t.cast("Pane", self), keys, **kwargs)
748+
def _run(argv: list[str], **kwargs: t.Any) -> subprocess.CompletedProcess[str]:
749+
calls.append(argv)
750+
return subprocess.CompletedProcess(argv, 0)
764751

765752
pane = FakePane()
766753
monkeypatch.setattr(io, "_get_server", lambda **kwargs: FakeServer())
767754
monkeypatch.setattr(io, "_resolve_pane", lambda *args, **kwargs: pane)
755+
monkeypatch.setattr("libtmux_mcp.tools.pane_tools.io.subprocess.run", _run)
768756

769757
async def _exercise() -> None:
770758
async with Client(_history_server("1")) as client:
@@ -793,11 +781,11 @@ async def _exercise() -> None:
793781
asyncio.run(_exercise())
794782

795783
assert calls == [
796-
("cmd", ("send-keys", "C-c")),
797-
("cmd", ("send-keys", "-l", "TUI_BATCH_DEFAULT")),
798-
("enter", ()),
799-
("cmd", ("send-keys", "-l", " batch-secret")),
800-
("enter", ()),
784+
["tmux", "send-keys", "-t", "%1", "--", "C-c"],
785+
["tmux", "send-keys", "-t", "%1", "-l", "--", "TUI_BATCH_DEFAULT"],
786+
["tmux", "send-keys", "-t", "%1", "Enter"],
787+
["tmux", "send-keys", "-t", "%1", "-l", "--", " batch-secret"],
788+
["tmux", "send-keys", "-t", "%1", "Enter"],
801789
]
802790

803791

@@ -806,8 +794,8 @@ def test_global_history_default_leaves_timed_batch_operations_explicit_only(
806794
) -> None:
807795
"""Timed batches preserve raw bytes and send Enter separately.
808796
809-
Timed batches bypass ``Pane.send_keys``, so subprocess interception at the
810-
pre-PTY argv boundary is required to expose prefixes and Enter coalescing.
797+
Subprocess interception at the pre-PTY argv boundary exposes prefixes
798+
and Enter coalescing.
811799
"""
812800
from libtmux_mcp.tools.pane_tools import io
813801

@@ -859,10 +847,10 @@ async def _exercise() -> None:
859847
asyncio.run(_exercise())
860848

861849
assert calls == [
862-
["tmux", "send-keys", "-t", "%1", "C-c"],
863-
["tmux", "send-keys", "-t", "%1", "-l", "TUI_BATCH_DEFAULT"],
850+
["tmux", "send-keys", "-t", "%1", "--", "C-c"],
851+
["tmux", "send-keys", "-t", "%1", "-l", "--", "TUI_BATCH_DEFAULT"],
864852
["tmux", "send-keys", "-t", "%1", "Enter"],
865-
["tmux", "send-keys", "-t", "%1", "-l", " batch-secret"],
853+
["tmux", "send-keys", "-t", "%1", "-l", "--", " batch-secret"],
866854
["tmux", "send-keys", "-t", "%1", "Enter"],
867855
]
868856

0 commit comments

Comments
 (0)