Skip to content

Commit 60a7381

Browse files
committed
fix(tools): Isolate paste_text tmux buffer and tighten temp-file lifecycle
why: paste_text had three related state-management issues on the same function: 1. Buffer clobber/leak. load-buffer with no -b wrote into tmux's default unnamed buffer. In any interactive session this overwrote whatever the operator had yanked. And if pane.cmd("paste-buffer", "-d", ...) failed for any reason (pane dead, server error), the -d never ran and the buffer leaked on the tmux server. 2. Temp-file disk leak on write failure. The with-block bound tmppath = f.name *after* f.write(text). If f.write raised (OSError disk full / quota), tmppath was never assigned, the subsequent try/finally block was never reached, and the temp file created by NamedTemporaryFile(delete=False) stayed on disk. 3. Silent stderr on subprocess failure. subprocess.run(..., check=True, capture_output=True) raised CalledProcessError with tmux's stderr attached, but the message agents saw was just "non-zero exit status 1" — no tmux diagnostic. what: - Generate a unique named buffer per call (mcp_paste_<uuid4 hex>) and load into that. paste-buffer uses -b NAME -d so the delete only affects the named buffer, leaving the user's unnamed buffer intact. - Bind tmppath = f.name BEFORE f.write so cleanup always has a path. - Use pathlib.Path(...).unlink(missing_ok=True) in finally. - Wrap delete-buffer in contextlib.suppress(Exception) as a defensive best-effort cleanup if paste-buffer raised before -d ran. - Catch CalledProcessError on load-buffer and surface tmux's stderr as a ToolError for actionable agent diagnostics. - Add imports: contextlib (stdlib, module top), uuid (stdlib, module top), namespace style per AGENTS.md §Imports. - Add test_paste_text_does_not_clobber_unnamed_buffer: pre-populates the unnamed buffer with a sentinel, calls paste_text, then asserts the unnamed buffer still contains the sentinel AND no mcp_paste_* named buffer leaks remain on the server.
1 parent 3e96084 commit 60a7381

2 files changed

Lines changed: 67 additions & 13 deletions

File tree

src/libtmux_mcp/tools/pane_tools.py

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22

33
from __future__ import annotations
44

5+
import contextlib
56
import pathlib
67
import re
78
import shlex
89
import typing as t
10+
import uuid
911

1012
from libtmux_mcp._utils import (
1113
ANNOTATIONS_CREATE,
@@ -1172,6 +1174,8 @@ def paste_text(
11721174
import subprocess
11731175
import tempfile
11741176

1177+
from fastmcp.exceptions import ToolError
1178+
11751179
server = _get_server(socket_name=socket_name)
11761180
pane = _resolve_pane(
11771181
server,
@@ -1181,31 +1185,49 @@ def paste_text(
11811185
window_id=window_id,
11821186
)
11831187

1184-
# Write text to a temp file and load into tmux buffer
1185-
# (libtmux's cmd() doesn't support stdin)
1186-
with tempfile.NamedTemporaryFile(mode="w", suffix=".txt", delete=False) as f:
1187-
f.write(text)
1188-
tmppath = f.name
1189-
1188+
# Use a unique named tmux buffer so we don't clobber the user's
1189+
# unnamed paste buffer, and so we can reliably clean up on error
1190+
# paths (paste-buffer -b NAME -d deletes the named buffer).
1191+
buffer_name = f"mcp_paste_{uuid.uuid4().hex}"
1192+
tmppath: str | None = None
11901193
try:
1191-
# Build tmux command args for loading buffer
1194+
# Write text to a temp file and load into tmux buffer
1195+
# (libtmux's cmd() doesn't support stdin).
1196+
with tempfile.NamedTemporaryFile(mode="w", suffix=".txt", delete=False) as f:
1197+
tmppath = f.name # bind first so cleanup works even if write fails
1198+
f.write(text)
1199+
1200+
# Build tmux command args for loading the named buffer
11921201
tmux_bin: str = getattr(server, "tmux_bin", None) or "tmux"
11931202
load_args: list[str] = [tmux_bin]
11941203
if server.socket_name:
11951204
load_args.extend(["-L", server.socket_name])
11961205
if server.socket_path:
11971206
load_args.extend(["-S", str(server.socket_path)])
1198-
load_args.extend(["load-buffer", tmppath])
1199-
subprocess.run(load_args, check=True, capture_output=True)
1200-
1201-
# Paste from buffer into pane
1202-
paste_args = ["-d"] # delete buffer after paste
1207+
load_args.extend(["load-buffer", "-b", buffer_name, tmppath])
1208+
1209+
try:
1210+
subprocess.run(load_args, check=True, capture_output=True)
1211+
except subprocess.CalledProcessError as e:
1212+
stderr = e.stderr.decode(errors="replace").strip() if e.stderr else ""
1213+
msg = f"load-buffer failed: {stderr or e}"
1214+
raise ToolError(msg) from e
1215+
1216+
# Paste from the named buffer. -d deletes only that named buffer,
1217+
# leaving any unnamed user buffer intact.
1218+
paste_args = ["-b", buffer_name, "-d"]
12031219
if bracket:
12041220
paste_args.append("-p") # bracketed paste mode
12051221
paste_args.extend(["-t", pane.pane_id or ""])
12061222
pane.cmd("paste-buffer", *paste_args)
12071223
finally:
1208-
pathlib.Path(tmppath).unlink()
1224+
if tmppath is not None:
1225+
pathlib.Path(tmppath).unlink(missing_ok=True)
1226+
# Defensive: the buffer should already be gone (paste-buffer -d
1227+
# deletes it), but if paste-buffer failed before -d took effect
1228+
# we leak an entry in the tmux server. Best-effort delete.
1229+
with contextlib.suppress(Exception):
1230+
server.cmd("delete-buffer", "-b", buffer_name)
12091231

12101232
return f"Text pasted to pane {pane.pane_id}"
12111233

tests/test_pane_tools.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -912,3 +912,35 @@ def test_paste_text(mcp_server: Server, mcp_pane: Pane) -> None:
912912
2,
913913
raises=True,
914914
)
915+
916+
917+
def test_paste_text_does_not_clobber_unnamed_buffer(
918+
mcp_server: Server, mcp_pane: Pane
919+
) -> None:
920+
"""paste_text must not overwrite the user's unnamed tmux paste buffer.
921+
922+
Regression guard for the pre-fix behavior: load-buffer without -b
923+
writes into tmux's default unnamed buffer, clobbering whatever the
924+
user had there. The fix uses a unique named buffer per call.
925+
"""
926+
sentinel = "USER_UNNAMED_BUFFER_SENTINEL_777"
927+
mcp_server.cmd("set-buffer", sentinel)
928+
929+
paste_text(
930+
text="echo BUFFER_ISOLATION_test",
931+
pane_id=mcp_pane.pane_id,
932+
socket_name=mcp_server.socket_name,
933+
)
934+
935+
# The user's unnamed buffer should still contain the sentinel.
936+
result = mcp_server.cmd("show-buffer")
937+
assert result.stdout and sentinel in "\n".join(result.stdout), (
938+
"paste_text clobbered the user's unnamed paste buffer"
939+
)
940+
941+
# And no mcp_paste_* named buffer should linger on the server.
942+
listing = mcp_server.cmd("list-buffers", "-F", "#{buffer_name}")
943+
buffer_names = "\n".join(listing.stdout or [])
944+
assert "mcp_paste_" not in buffer_names, (
945+
f"paste_text leaked a named buffer: {buffer_names!r}"
946+
)

0 commit comments

Comments
 (0)