Skip to content

Commit 6c80021

Browse files
committed
fix(Session.attach): Remove refresh() call that fails after session killed
When a user attaches to a tmux session via `tmuxp load`, works in the session, then kills it (e.g., closes all windows) before detaching, the Session.attach() method would raise TmuxObjectDoesNotExist. This happened because attach() called self.refresh() after the attach-session command returned. Since attach-session is a blocking interactive command, the session state can change arbitrarily during attachment - including being killed entirely. The refresh() call was semantically incorrect for interactive commands: - attach-session blocks until user detaches - Session state can change during attachment - Refreshing after such a command makes no sense Introduced in: 9a5147a (feat(Session.attach): Add Session.attach()) Triggered by: tmuxp fdafdd2b switching from attach_session() to attach() Adds regression test using NamedTuple + parametrize + test_id pattern.
1 parent 64276e4 commit 6c80021

File tree

2 files changed

+84
-2
lines changed

2 files changed

+84
-2
lines changed

src/libtmux/session.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,8 +329,6 @@ def attach(
329329
if proc.stderr:
330330
raise exc.LibTmuxException(proc.stderr)
331331

332-
self.refresh()
333-
334332
return self
335333

336334
def kill(

tests/test_session.py

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,3 +481,87 @@ def test_new_window_start_directory_pathlib(
481481
actual_path = str(pathlib.Path(active_pane.pane_current_path).resolve())
482482
expected_path = str(user_path.resolve())
483483
assert actual_path == expected_path
484+
485+
486+
class SessionAttachRefreshFixture(t.NamedTuple):
487+
"""Test fixture for Session.attach() refresh behavior regression.
488+
489+
This tests the scenario where a session is killed while the user is attached,
490+
and then attach() tries to call refresh() which fails because the session
491+
no longer exists.
492+
493+
See: https://github.com/tmux-python/tmuxp/issues/1002
494+
"""
495+
496+
test_id: str
497+
kill_session_before_refresh: bool
498+
expect_no_exception: bool
499+
500+
501+
SESSION_ATTACH_REFRESH_FIXTURES: list[SessionAttachRefreshFixture] = [
502+
SessionAttachRefreshFixture(
503+
test_id="session_killed_during_attach_should_not_raise",
504+
kill_session_before_refresh=True,
505+
expect_no_exception=True, # attach() should NOT raise if session gone
506+
),
507+
]
508+
509+
510+
@pytest.mark.parametrize(
511+
list(SessionAttachRefreshFixture._fields),
512+
SESSION_ATTACH_REFRESH_FIXTURES,
513+
ids=[test.test_id for test in SESSION_ATTACH_REFRESH_FIXTURES],
514+
)
515+
def test_session_attach_does_not_fail_if_session_killed_during_attach(
516+
server: Server,
517+
monkeypatch: pytest.MonkeyPatch,
518+
test_id: str,
519+
kill_session_before_refresh: bool,
520+
expect_no_exception: bool,
521+
) -> None:
522+
"""Regression test: Session.attach() should not fail if session is killed.
523+
524+
When a user is attached to a tmux session via `tmuxp load`, then kills the
525+
session from within tmux (e.g., kills all windows), and then detaches,
526+
the attach() method should not raise an exception.
527+
528+
Currently, attach() calls self.refresh() after attach-session returns, which
529+
fails with TmuxObjectDoesNotExist if the session no longer exists.
530+
531+
The fix is to remove the refresh() call from attach() since:
532+
1. attach-session is a blocking interactive command
533+
2. Session state can change arbitrarily while the user is attached
534+
3. Refreshing after such a command makes no semantic sense
535+
"""
536+
from libtmux.common import tmux_cmd
537+
538+
# Create a new session specifically for this test
539+
test_session = server.new_session(detach=True)
540+
541+
# Store original cmd method
542+
original_cmd = test_session.cmd
543+
544+
# Create a mock tmux_cmd result that simulates successful attach-session
545+
class MockTmuxCmd:
546+
def __init__(self) -> None:
547+
self.stdout: list[str] = []
548+
self.stderr: list[str] = []
549+
self.cmd: list[str] = ["tmux", "attach-session"]
550+
551+
def patched_cmd(cmd_name: str, *args: t.Any, **kwargs: t.Any) -> tmux_cmd:
552+
"""Patched cmd that kills session after attach-session."""
553+
if cmd_name == "attach-session" and kill_session_before_refresh:
554+
# Simulate: attach-session succeeded, user worked, then killed session
555+
# This happens BEFORE refresh() is called
556+
test_session.kill()
557+
return MockTmuxCmd() # type: ignore[return-value]
558+
return original_cmd(cmd_name, *args, **kwargs)
559+
560+
monkeypatch.setattr(test_session, "cmd", patched_cmd)
561+
562+
# This should NOT raise an exception - the session being killed during
563+
# attachment is a valid scenario that should be handled gracefully
564+
if expect_no_exception:
565+
# After the fix (removing refresh()), this should not raise
566+
test_session.attach()
567+
# Test passes if no exception is raised

0 commit comments

Comments
 (0)