Skip to content

Commit 9adf370

Browse files
committed
mcp(fix[wait_for]): Release an abandoned waiter
why: A killed wait-for client stays queued on its channel in tmux, so the next signal was spent on the dead client and a later wait on that channel timed out. libtmux's Server.wait_for already releases the waiter by signalling it while alive. what: - Add release_argv to _run_tmux_bounded: expiry and cancellation signal the channel, wait for the client to exit, and kill it only if it does not - Pass it from wait_for_channel and run_command - Test signal-then-wait after a timeout and after a cancel; both fail when release_argv is not passed
1 parent 7aa0545 commit 9adf370

5 files changed

Lines changed: 125 additions & 10 deletions

File tree

‎CHANGES‎

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

99
### Fixes
1010

11+
**A timed-out or cancelled wait no longer swallows the channel's next signal**
12+
13+
`wait_for_channel` and `run_command` used to kill the waiting tmux client when
14+
they gave up, which left it queued on the channel and spent the next signal on
15+
it. They now release the waiter first, so signalling the channel and waiting
16+
again succeeds.
17+
1118
**`send_keys_batch`, `paste_text` and the buffer tools use libtmux's safe commands**
1219

1320
Text that starts with `-` or ends with `;` reaches the pane verbatim in a timed

‎src/libtmux_mcp/_tmux_proc.py‎

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@
3131
#: loop's child watcher reaps the pid whether or not we wait.
3232
_TMUX_REAP_SECONDS = 0.5
3333

34+
#: Bound on each half of a waiter release: the ``wait-for -S`` that frees
35+
#: the channel, then the waiter's own exit.
36+
_RELEASE_GRACE_SECONDS = 1.0
37+
3438

3539
async def _kill_and_reap(
3640
proc: asyncio.subprocess.Process, task: asyncio.Future[t.Any]
@@ -62,8 +66,63 @@ async def _kill_and_reap(
6266
await asyncio.wait_for(proc.wait(), timeout=_TMUX_REAP_SECONDS)
6367

6468

69+
async def _release_waiter(
70+
proc: asyncio.subprocess.Process,
71+
task: asyncio.Future[t.Any],
72+
release_argv: list[str],
73+
) -> None:
74+
"""End a ``wait-for`` client without leaving a ghost waiter.
75+
76+
tmux keeps a killed waiter queued on its channel and only remembers a
77+
signal while nobody waits, so the channel's next signal would be spent
78+
on the dead client. Signalling while the client is still alive makes
79+
tmux dequeue it, leaving the channel clean. The client is killed only
80+
when it does not exit, which is when the server is not answering.
81+
Mirrors ``libtmux.common._release_waiter``.
82+
83+
Parameters
84+
----------
85+
proc : asyncio.subprocess.Process
86+
The ``tmux wait-for`` client.
87+
task : asyncio.Future
88+
The in-flight ``proc.communicate()`` future.
89+
release_argv : list of str
90+
Full command line that signals the waiter's channel.
91+
"""
92+
with contextlib.suppress(OSError, TimeoutError):
93+
releaser = await asyncio.create_subprocess_exec(
94+
*release_argv,
95+
stdout=asyncio.subprocess.DEVNULL,
96+
stderr=asyncio.subprocess.DEVNULL,
97+
)
98+
try:
99+
await asyncio.wait_for(releaser.wait(), timeout=_RELEASE_GRACE_SECONDS)
100+
except BaseException:
101+
with contextlib.suppress(ProcessLookupError):
102+
releaser.kill()
103+
raise
104+
with contextlib.suppress(TimeoutError):
105+
await asyncio.wait_for(asyncio.shield(task), timeout=_RELEASE_GRACE_SECONDS)
106+
await _kill_and_reap(proc, task)
107+
108+
109+
async def _end_child(
110+
proc: asyncio.subprocess.Process,
111+
task: asyncio.Future[t.Any],
112+
release_argv: list[str] | None,
113+
) -> None:
114+
"""Release a ``wait-for`` waiter, or kill any other child."""
115+
if release_argv is None:
116+
await _kill_and_reap(proc, task)
117+
else:
118+
await _release_waiter(proc, task, release_argv)
119+
120+
65121
async def _run_tmux_bounded(
66-
argv: list[str], *, timeout: float
122+
argv: list[str],
123+
*,
124+
timeout: float,
125+
release_argv: list[str] | None = None,
67126
) -> tuple[int, bytes, bytes]:
68127
"""Run one tmux argv under a hard bound, killing it on cancellation.
69128
@@ -75,6 +134,10 @@ async def _run_tmux_bounded(
75134
timeout : float
76135
Wall-clock bound in seconds. On expiry the child is killed and
77136
reaped before ``TimeoutError`` is raised.
137+
release_argv : list of str, optional
138+
For a ``wait-for`` client: the command that signals its channel.
139+
When given, expiry and cancellation release the waiter through
140+
:func:`_release_waiter` instead of killing it outright.
78141
79142
Returns
80143
-------
@@ -105,7 +168,7 @@ async def _run_tmux_bounded(
105168
try:
106169
done, _pending = await asyncio.wait({task}, timeout=timeout)
107170
if not done:
108-
await _kill_and_reap(proc, task)
171+
await _end_child(proc, task, release_argv)
109172
raise TimeoutError
110173
stdout, stderr = task.result()
111174
except asyncio.CancelledError:
@@ -115,7 +178,7 @@ async def _run_tmux_bounded(
115178
# above just as often as on ``task.result()``, so the guard
116179
# must span both — otherwise a cancel while waiting orphans the
117180
# child.
118-
await _kill_and_reap(proc, task)
181+
await _end_child(proc, task, release_argv)
119182
raise
120183
assert proc.returncode is not None
121184
return proc.returncode, stdout, stderr

‎src/libtmux_mcp/tools/pane_tools/io.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,9 @@ async def run_command(
428428
stderr_bytes = b""
429429
try:
430430
returncode, _stdout, stderr_bytes = await _run_tmux_bounded(
431-
wait_argv, timeout=effective_timeout
431+
wait_argv,
432+
timeout=effective_timeout,
433+
release_argv=_tmux_argv(server, "wait-for", "-S", channel),
432434
)
433435
except TimeoutError:
434436
timed_out = True

‎src/libtmux_mcp/tools/wait_for_tools.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ async def wait_for_channel(
204204
cname = _validate_channel_name(channel)
205205
effective_timeout = min(timeout, _wait_ceiling_seconds())
206206
argv = _tmux_argv(server, "wait-for", cname)
207+
release_argv = _tmux_argv(server, "wait-for", "-S", cname)
207208
# FastMCP direct-awaits async tools on its event loop, and ``tmux
208209
# wait-for`` blocks for the full timeout, so the child must not run
209210
# on the loop. It must not run on a worker thread either:
@@ -215,7 +216,7 @@ async def wait_for_channel(
215216
# :func:`~libtmux_mcp.tools.pane_tools.wait.wait_for_text` uses.
216217
try:
217218
returncode, _stdout, stderr = await _run_tmux_bounded(
218-
argv, timeout=effective_timeout
219+
argv, timeout=effective_timeout, release_argv=release_argv
219220
)
220221
except TimeoutError as e:
221222
msg = (

‎tests/test_wait_for_tools.py‎

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -458,11 +458,9 @@ def test_wait_for_channel_kills_tmux_child_on_cancel(mcp_server: Server) -> None
458458
child alive another 13 s; through a real agent TUI with the ceiling
459459
raised to 120 s and a 90 s timeout, ~61 s past the user's Esc.
460460
461-
Note the harm is the live process itself, not a stolen signal —
462-
tmux keeps the server-side waiter registered even after the client
463-
dies (verified against ``tmux wait-for``), so ``wait-for -S`` is
464-
swallowed either way. Only the process is observable, so that is
465-
what this asserts.
461+
The channel itself is checked by
462+
``test_wait_for_channel_leaves_channel_reusable``; this one asserts the
463+
process is gone.
466464
"""
467465
channel = "wf_cancel_reap_test"
468466
socket_name = mcp_server.socket_name
@@ -502,3 +500,47 @@ async def _drive() -> list[int]:
502500
f"cancelled wait_for_channel orphaned tmux child(ren) {survivors}; "
503501
"the child outlives the cancellation for the rest of its timeout"
504502
)
503+
504+
505+
@pytest.mark.parametrize("how", ["timeout", "cancel"])
506+
@pytest.mark.usefixtures("mcp_session")
507+
def test_wait_for_channel_leaves_channel_reusable(mcp_server: Server, how: str) -> None:
508+
"""An abandoned wait must not leave a ghost waiter on the channel.
509+
510+
tmux remembers a signal only while nobody waits, and a killed client
511+
stays queued as a waiter, so the next signal was spent on the dead
512+
client: a later wait on the same channel then timed out although the
513+
channel had been signalled first. Releasing the waiter by signalling
514+
it while alive dequeues it, so signal-then-wait returns at once.
515+
"""
516+
channel = f"wf_ghost_{how}"
517+
socket_name = mcp_server.socket_name
518+
assert socket_name is not None
519+
520+
async def _abandon() -> None:
521+
if how == "timeout":
522+
with pytest.raises(ToolError, match="timeout"):
523+
await wait_for_channel(
524+
channel=channel, timeout=0.3, socket_name=socket_name
525+
)
526+
return
527+
task = asyncio.create_task(
528+
wait_for_channel(channel=channel, timeout=8.0, socket_name=socket_name)
529+
)
530+
await asyncio.sleep(0.5)
531+
task.cancel()
532+
with pytest.raises(asyncio.CancelledError):
533+
await task
534+
535+
async def _drive() -> str:
536+
await _abandon()
537+
await signal_channel(channel=channel, socket_name=socket_name)
538+
return await wait_for_channel(
539+
channel=channel, timeout=2.0, socket_name=socket_name
540+
)
541+
542+
started = time.monotonic()
543+
result = asyncio.run(_drive())
544+
545+
assert "was signalled" in result
546+
assert time.monotonic() - started < 1.9

0 commit comments

Comments
 (0)