Skip to content

Commit a74c602

Browse files
committed
Server(fix[new_session]): Harden error handling and TMUX env restoration
why: new_session() deletes os.environ["TMUX"] before calling tmux but only restores it on the success path — any exception permanently leaks the deletion. Additionally, proc.stdout[0] raises an unhelpful IndexError if tmux produces no output. what: - Wrap all code after del os.environ["TMUX"] in try/finally to ensure restoration on any exception (including setup-phase errors) - Guard against empty proc.stdout with descriptive LibTmuxException - Add tests for empty stdout, TMUX env restoration on cmd errors, and TMUX env restoration on setup-phase errors - Document monkeypatch usage in test docstrings per CLAUDE.md
1 parent 9da19ba commit a74c602

2 files changed

Lines changed: 123 additions & 28 deletions

File tree

src/libtmux/server.py

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -539,46 +539,51 @@ def new_session(
539539
if env:
540540
del os.environ["TMUX"]
541541

542-
tmux_args: tuple[str | int, ...] = (
543-
"-P",
544-
"-F#{session_id}", # output
545-
)
542+
try:
543+
tmux_args: tuple[str | int, ...] = (
544+
"-P",
545+
"-F#{session_id}", # output
546+
)
546547

547-
if session_name is not None:
548-
tmux_args += (f"-s{session_name}",)
548+
if session_name is not None:
549+
tmux_args += (f"-s{session_name}",)
549550

550-
if not attach:
551-
tmux_args += ("-d",)
551+
if not attach:
552+
tmux_args += ("-d",)
552553

553-
if start_directory:
554-
start_directory = pathlib.Path(start_directory).expanduser()
555-
tmux_args += ("-c", str(start_directory))
554+
if start_directory:
555+
start_directory = pathlib.Path(start_directory).expanduser()
556+
tmux_args += ("-c", str(start_directory))
556557

557-
if window_name:
558-
tmux_args += ("-n", window_name)
558+
if window_name:
559+
tmux_args += ("-n", window_name)
559560

560-
if x is not None:
561-
tmux_args += ("-x", x)
561+
if x is not None:
562+
tmux_args += ("-x", x)
562563

563-
if y is not None:
564-
tmux_args += ("-y", y)
564+
if y is not None:
565+
tmux_args += ("-y", y)
565566

566-
if environment:
567-
for k, v in environment.items():
568-
tmux_args += (f"-e{k}={v}",)
567+
if environment:
568+
for k, v in environment.items():
569+
tmux_args += (f"-e{k}={v}",)
569570

570-
if window_command:
571-
tmux_args += (window_command,)
571+
if window_command:
572+
tmux_args += (window_command,)
572573

573-
proc = self.cmd("new-session", *tmux_args)
574+
proc = self.cmd("new-session", *tmux_args)
574575

575-
if proc.stderr:
576-
raise exc.LibTmuxException(proc.stderr)
576+
if proc.stderr:
577+
raise exc.LibTmuxException(proc.stderr)
577578

578-
session_stdout = proc.stdout[0]
579+
if not proc.stdout:
580+
msg = "new-session produced no output"
581+
raise exc.LibTmuxException(msg)
579582

580-
if env:
581-
os.environ["TMUX"] = env
583+
session_stdout = proc.stdout[0]
584+
finally:
585+
if env:
586+
os.environ["TMUX"] = env
582587

583588
session_formatters = dict(
584589
zip(

tests/test_server.py

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
import pytest
1313

14+
from libtmux import exc
1415
from libtmux.server import Server
1516

1617
if t.TYPE_CHECKING:
@@ -104,6 +105,95 @@ def test_new_session(server: Server) -> None:
104105
assert server.has_session("test_new_session")
105106

106107

108+
def test_new_session_empty_stdout(
109+
server: Server,
110+
monkeypatch: pytest.MonkeyPatch,
111+
) -> None:
112+
"""Server.new_session raises LibTmuxException on empty stdout.
113+
114+
Uses monkeypatch to simulate empty stdout, which cannot be triggered
115+
through normal tmux operations.
116+
"""
117+
original_cmd = server.cmd
118+
119+
def mock_cmd(cmd: str, *args: t.Any, **kwargs: t.Any) -> t.Any:
120+
result = original_cmd(cmd, *args, **kwargs)
121+
if cmd == "new-session":
122+
result.stdout = []
123+
return result
124+
125+
monkeypatch.setattr(server, "cmd", mock_cmd)
126+
127+
with pytest.raises(exc.LibTmuxException, match="new-session produced no output"):
128+
server.new_session(session_name="test_empty_stdout")
129+
130+
monkeypatch.undo()
131+
if server.has_session("test_empty_stdout"):
132+
server.kill_session("test_empty_stdout")
133+
134+
135+
def test_new_session_restores_tmux_env_on_error(
136+
server: Server,
137+
monkeypatch: pytest.MonkeyPatch,
138+
) -> None:
139+
"""Server.new_session restores TMUX env var even when an exception is raised.
140+
141+
Uses monkeypatch to simulate empty stdout, which cannot be triggered
142+
through normal tmux operations.
143+
"""
144+
original_cmd = server.cmd
145+
sentinel = "/tmp/libtmux-test-fake-socket,12345,0"
146+
147+
monkeypatch.setenv("TMUX", sentinel)
148+
149+
def mock_cmd(cmd: str, *args: t.Any, **kwargs: t.Any) -> t.Any:
150+
result = original_cmd(cmd, *args, **kwargs)
151+
if cmd == "new-session":
152+
result.stdout = []
153+
return result
154+
155+
monkeypatch.setattr(server, "cmd", mock_cmd)
156+
157+
with pytest.raises(exc.LibTmuxException, match="new-session produced no output"):
158+
server.new_session(session_name="test_env_restore")
159+
160+
assert os.environ.get("TMUX") == sentinel
161+
162+
monkeypatch.undo()
163+
if server.has_session("test_env_restore"):
164+
server.kill_session("test_env_restore")
165+
166+
167+
def test_new_session_restores_tmux_env_on_setup_error(
168+
server: Server,
169+
monkeypatch: pytest.MonkeyPatch,
170+
tmp_path: pathlib.Path,
171+
) -> None:
172+
"""Server.new_session restores TMUX env var when setup code before cmd() raises.
173+
174+
Uses monkeypatch to make pathlib.Path.expanduser raise, which cannot be
175+
triggered through normal tmux operations. This verifies the try/finally
176+
covers the arg-building phase, not just the cmd() call.
177+
"""
178+
sentinel = "/tmp/libtmux-test-fake-socket,99999,0"
179+
180+
monkeypatch.setenv("TMUX", sentinel)
181+
182+
def broken_expanduser(self: pathlib.Path) -> t.Any:
183+
msg = "injected setup error"
184+
raise RuntimeError(msg)
185+
186+
monkeypatch.setattr(pathlib.Path, "expanduser", broken_expanduser)
187+
188+
with pytest.raises(RuntimeError, match="injected setup error"):
189+
server.new_session(
190+
session_name="test_setup_error",
191+
start_directory=tmp_path,
192+
)
193+
194+
assert os.environ.get("TMUX") == sentinel
195+
196+
107197
def test_new_session_no_name(server: Server) -> None:
108198
"""Server.new_session works with no name."""
109199
first_session = server.new_session()

0 commit comments

Comments
 (0)