Skip to content

Commit c8dae37

Browse files
committed
WorkspaceBuilder(fix[reused_sessions]): Keep existing session hooks and env intact
why: append and --here reuse a live tmux session rather than creating a tmuxp owned session. Writing lifecycle hooks or stop metadata onto that reused session can overwrite unrelated teardown behavior, and copying first-pane environment into session state makes later windows inherit variables they were never meant to see. what: - limit on_project_exit, on_project_stop, and start_directory session metadata to sessions created by the current build - keep --here first-pane provisioning local to respawn-pane instead of the session environment - add reused-session and non-leaking here-mode tests around hooks and env
1 parent 1a82b3a commit c8dae37

2 files changed

Lines changed: 122 additions & 29 deletions

File tree

src/tmuxp/workspace/builder.py

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -568,8 +568,10 @@ def build(
568568
for option, value in self.session_config["environment"].items():
569569
self.session.set_environment(option, value)
570570

571-
# Set lifecycle tmux hooks
572-
if "on_project_exit" in self.session_config:
571+
# Session-scoped lifecycle hooks and metadata belong only to sessions
572+
# created by this build. Reused sessions may already carry unrelated
573+
# hooks or tmuxp metadata from other windows/workspaces.
574+
if session_created and "on_project_exit" in self.session_config:
573575
exit_cmds = self.session_config["on_project_exit"]
574576
if isinstance(exit_cmds, str):
575577
exit_cmds = [exit_cmds]
@@ -585,7 +587,7 @@ def build(
585587
)
586588

587589
# Store on_project_stop in session environment for tmuxp stop
588-
if "on_project_stop" in self.session_config:
590+
if session_created and "on_project_stop" in self.session_config:
589591
stop_cmds = self.session_config["on_project_stop"]
590592
if isinstance(stop_cmds, str):
591593
stop_cmds = [stop_cmds]
@@ -595,7 +597,7 @@ def build(
595597
)
596598

597599
# Store start_directory in session environment for hook cwd
598-
if "start_directory" in self.session_config:
600+
if session_created and "start_directory" in self.session_config:
599601
self.session.set_environment(
600602
"TMUXP_START_DIRECTORY",
601603
self.session_config["start_directory"],
@@ -702,15 +704,9 @@ def iter_create_windows(
702704
if panes and "start_directory" in panes[0]:
703705
start_directory = panes[0]["start_directory"]
704706

705-
# Provision environment via tmux session env (inherited
706-
# by new panes). Matches teamocil, which does not inject
707-
# env vars via send_keys at all.
708707
environment = window_config.get("environment")
709708
if panes and "environment" in panes[0]:
710709
environment = panes[0]["environment"]
711-
if environment:
712-
for _ekey, _eval in environment.items():
713-
session.set_environment(_ekey, str(_eval))
714710

715711
# Resolve window_shell
716712
window_shell = window_config.get("window_shell")

tests/workspace/test_builder.py

Lines changed: 116 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -605,17 +605,19 @@ def test_here_mode_duplicate_session_name(
605605
def test_here_mode_provisions_environment(
606606
session: Session,
607607
) -> None:
608-
"""--here mode sets environment via session and respawn-pane, not send_keys."""
609-
from libtmux.test.retry import retry_until
610-
608+
"""--here mode provisions the reused pane without mutating session env."""
611609
workspace: dict[str, t.Any] = {
612610
"session_name": session.name,
613611
"windows": [
614612
{
615613
"window_name": "env-test",
616614
"environment": {"TMUXP_HERE_TEST": "hello_here"},
617615
"panes": [
618-
{"shell_command": ["echo $TMUXP_HERE_TEST"]},
616+
{
617+
"shell_command": [
618+
"printf '%s' \"${TMUXP_HERE_TEST-unset}\"",
619+
],
620+
},
619621
],
620622
},
621623
],
@@ -625,11 +627,9 @@ def test_here_mode_provisions_environment(
625627
builder = WorkspaceBuilder(session_config=workspace, server=session.server)
626628
builder.build(session=session, here=True)
627629

628-
# Verify env var is set at session level (tmux primitive)
629630
env = session.show_environment()
630-
assert env.get("TMUXP_HERE_TEST") == "hello_here"
631+
assert env.get("TMUXP_HERE_TEST") is None
631632

632-
# Verify the respawned pane also sees the var
633633
pane = session.active_window.active_pane
634634
assert pane is not None
635635

@@ -747,25 +747,44 @@ def test_here_mode_respawn_provisioning(
747747

748748
if environment:
749749
env = session.show_environment()
750-
for key, val in environment.items():
751-
assert env.get(key) == val
750+
for key in environment:
751+
assert env.get(key) is None
752752

753753

754-
def test_here_mode_respawn_multiple_env_vars(
754+
def test_here_mode_does_not_leak_first_pane_environment(
755755
session: Session,
756756
) -> None:
757-
"""--here mode sets multiple environment variables via set_environment."""
757+
"""--here mode keeps first-pane environment out of later windows."""
758758
workspace: dict[str, t.Any] = {
759759
"session_name": session.name,
760760
"windows": [
761761
{
762-
"window_name": "multi-env",
762+
"window_name": "first-window",
763763
"environment": {
764-
"TMUXP_A": "alpha",
765-
"TMUXP_B": "bravo",
766-
"TMUXP_C": "charlie",
764+
"TMUXP_HERE_ALPHA": "alpha",
765+
"TMUXP_HERE_BRAVO": "bravo",
766+
"TMUXP_HERE_CHARLIE": "charlie",
767767
},
768-
"panes": [{"shell_command": []}],
768+
"panes": [
769+
{
770+
"shell_command": [
771+
"printf '%s:%s:%s' "
772+
'"$TMUXP_HERE_ALPHA" '
773+
'"$TMUXP_HERE_BRAVO" '
774+
'"$TMUXP_HERE_CHARLIE"',
775+
],
776+
},
777+
],
778+
},
779+
{
780+
"window_name": "later-window",
781+
"panes": [
782+
{
783+
"shell_command": [
784+
"printf '%s' \"${TMUXP_HERE_ALPHA-unset}\"",
785+
],
786+
},
787+
],
769788
},
770789
],
771790
}
@@ -775,9 +794,87 @@ def test_here_mode_respawn_multiple_env_vars(
775794
builder.build(session=session, here=True)
776795

777796
env = session.show_environment()
778-
assert env.get("TMUXP_A") == "alpha"
779-
assert env.get("TMUXP_B") == "bravo"
780-
assert env.get("TMUXP_C") == "charlie"
797+
assert env.get("TMUXP_HERE_ALPHA") is None
798+
assert env.get("TMUXP_HERE_BRAVO") is None
799+
assert env.get("TMUXP_HERE_CHARLIE") is None
800+
801+
first_window = next(
802+
window for window in session.windows if window.name == "first-window"
803+
)
804+
later_window = next(
805+
window for window in session.windows if window.name == "later-window"
806+
)
807+
first_pane = first_window.active_pane
808+
later_pane = later_window.active_pane
809+
assert first_pane is not None
810+
assert later_pane is not None
811+
812+
assert retry_until(
813+
lambda: "alpha:bravo:charlie" in "\n".join(first_pane.capture_pane()),
814+
seconds=5,
815+
)
816+
assert retry_until(
817+
lambda: "unset" in "\n".join(later_pane.capture_pane()),
818+
seconds=5,
819+
)
820+
821+
822+
class ReusedSessionMetadataFixture(t.NamedTuple):
823+
"""Fixture for reused-session lifecycle metadata scenarios."""
824+
825+
test_id: str
826+
build_kwargs: dict[str, bool]
827+
828+
829+
REUSED_SESSION_METADATA_FIXTURES: list[ReusedSessionMetadataFixture] = [
830+
ReusedSessionMetadataFixture(
831+
test_id="append",
832+
build_kwargs={"append": True},
833+
),
834+
ReusedSessionMetadataFixture(
835+
test_id="here",
836+
build_kwargs={"here": True},
837+
),
838+
]
839+
840+
841+
@pytest.mark.parametrize(
842+
list(ReusedSessionMetadataFixture._fields),
843+
REUSED_SESSION_METADATA_FIXTURES,
844+
ids=[fixture.test_id for fixture in REUSED_SESSION_METADATA_FIXTURES],
845+
)
846+
def test_reused_session_keeps_existing_lifecycle_metadata(
847+
session: Session,
848+
tmp_path: pathlib.Path,
849+
test_id: str,
850+
build_kwargs: dict[str, bool],
851+
) -> None:
852+
"""Append and --here preserve pre-existing session hook and env metadata."""
853+
original_hook = "run-shell 'printf %s original-exit >/dev/null'"
854+
session.set_hook("client-detached", original_hook)
855+
session.set_environment("TMUXP_ON_PROJECT_STOP", "original stop")
856+
session.set_environment("TMUXP_START_DIRECTORY", "/original/start")
857+
858+
workspace: dict[str, t.Any] = {
859+
"session_name": session.name,
860+
"start_directory": str(tmp_path),
861+
"on_project_exit": "printf '%s' new-exit >/dev/null",
862+
"on_project_stop": "printf '%s' new-stop >/dev/null",
863+
"windows": [
864+
{"window_name": f"{test_id}-window", "panes": [{"shell_command": []}]}
865+
],
866+
}
867+
workspace = loader.expand(workspace)
868+
workspace = loader.trickle(workspace)
869+
870+
builder = WorkspaceBuilder(session_config=workspace, server=session.server)
871+
builder.build(session=session, **build_kwargs)
872+
873+
hooks = [str(value) for value in session.show_hooks().values()]
874+
assert any("original-exit" in value for value in hooks)
875+
assert all("new-exit" not in value for value in hooks)
876+
assert session.getenv("TMUXP_ON_PROJECT_STOP") == "original stop"
877+
assert session.getenv("TMUXP_START_DIRECTORY") == "/original/start"
781878

782879

783880
def test_here_mode_respawn_warns_on_running_processes(

0 commit comments

Comments
 (0)