Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 0 additions & 111 deletions .pr/design.html

This file was deleted.

47 changes: 20 additions & 27 deletions openhands-agent-server/openhands/agent_server/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,33 +133,26 @@ def _ensure_server_tmux_tmpdir() -> tuple[Path, bool]:


def _cleanup_stale_tmux_sessions() -> None:
"""Clean up any stale tmux sessions on server startup.

Tmux sessions live in a separate process that survives agent-server restarts.
This function kills all existing sessions on the shared OpenHands tmux socket
to prevent accumulation of orphaned sessions.
"""
try:
server = libtmux.Server(socket_name=TMUX_SOCKET_NAME)
sessions = server.sessions
if not sessions:
logger.debug("No tmux sessions found on %s socket", TMUX_SOCKET_NAME)
return

logger.info("Cleaning up %d stale tmux session(s) on startup", len(sessions))

for session in sessions:
try:
logger.debug("Killing tmux session: %s", session.name)
session.kill()
except Exception as e:
logger.warning("Failed to kill tmux session %s: %s", session.name, e)

logger.info("Tmux cleanup completed")

except Exception as e:
# Don't let tmux cleanup failures prevent server startup
logger.warning("Failed to cleanup tmux sessions: %s", e)
"""Clean up legacy and isolated terminal sockets on server startup."""
if os.name != "posix":
return
socket_dir = Path(os.environ.get("TMUX_TMPDIR", "/tmp")) / f"tmux-{os.getuid()}"
socket_names = [TMUX_SOCKET_NAME]
socket_names.extend(
path.name
for path in socket_dir.glob(f"{TMUX_SOCKET_NAME}-" + "[0-9a-f]" * 32)
if path.is_socket()
)
for socket_name in socket_names:
try:
server = libtmux.Server(socket_name=socket_name)
for session in server.sessions:
try:
session.kill()
except Exception as e:
logger.warning("Failed to kill tmux session %s: %s", session, e)
except Exception as e:
logger.warning("Failed to cleanup tmux socket %s: %s", socket_name, e)


@asynccontextmanager
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,10 @@ def initialize(self) -> None:
return

env = build_terminal_env(self.env)
self._server = libtmux.Server(socket_name=TMUX_SOCKET_NAME, environment=env)
# Stale handles must not reach another pool after tmux restarts.
self._server = libtmux.Server(
socket_name=f"{TMUX_SOCKET_NAME}-{uuid.uuid4().hex}", environment=env
)
session_name = f"openhands-pool-{self.username}-{uuid.uuid4()}"
self._session = self._server.new_session(
session_name=session_name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,10 @@ def initialize(self) -> None:
# auto-launch `less` on a TTY don't capture the pane and wedge the session.
env.setdefault("GIT_PAGER", "cat")
env.setdefault("PAGER", "cat")
# Use a dedicated socket to isolate OpenHands sessions from the user's tmux
self.server = libtmux.Server(socket_name=TMUX_SOCKET_NAME, environment=env)
# Never reuse a socket: tmux numeric IDs restart at zero after server exit.
self.server = libtmux.Server(
socket_name=f"{TMUX_SOCKET_NAME}-{uuid.uuid4().hex}", environment=env
)
_shell_command = "/bin/bash"
if self.username in ["root", "openhands"]:
# This starts a non-login (new) shell for the given user
Expand Down
22 changes: 22 additions & 0 deletions tests/agent_server/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@
from types import SimpleNamespace
from unittest.mock import AsyncMock, patch

import libtmux
import pytest
from fastapi.testclient import TestClient

from openhands.agent_server.api import (
_cleanup_stale_tmux_sessions,
_default_server_tmux_tmpdir,
_ensure_server_tmux_tmpdir,
_get_root_path,
Expand Down Expand Up @@ -61,6 +63,26 @@ def test_ensure_server_tmux_tmpdir_respects_existing_env(tmp_path, monkeypatch):
assert not existing.exists()


@pytest.mark.skipif(os.name != "posix", reason="tmux requires Unix")
def test_cleanup_stale_tmux_sessions_includes_isolated_sockets(monkeypatch):
with tempfile.TemporaryDirectory(prefix="oh-cleanup-") as socket_dir:
monkeypatch.setenv("TMUX_TMPDIR", socket_dir)
servers = [
libtmux.Server(socket_name=name)
for name in ["openhands", "openhands-" + "a" * 32, "unrelated"]
]
try:
for server in servers:
server.new_session(session_name="test")
_cleanup_stale_tmux_sessions()
assert not servers[0].sessions
assert not servers[1].sessions
assert servers[2].sessions
finally:
for server in servers:
server.cmd("kill-server")


class TestStaticFilesServing:
"""Test static files serving functionality."""

Expand Down
42 changes: 42 additions & 0 deletions tests/tools/terminal/test_tmux_pane_pool.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
TMUX_SESSION_WIDTH,
)
from openhands.tools.terminal.terminal.tmux_pane_pool import TmuxPanePool
from openhands.tools.terminal.terminal.tmux_terminal import TmuxTerminal


def test_tmux_session_viewport_is_bounded():
Expand Down Expand Up @@ -255,3 +256,44 @@ def test_checkin_foreign_pane_is_ignored(pool):

fake = TmuxTerminal.__new__(TmuxTerminal)
pool.checkin(fake) # should log warning, not crash


@pytest.mark.parametrize("pooled", [True, False])
@pytest.mark.parametrize("operation", ["read", "write", "close"])
def test_stale_terminal_cannot_access_restarted_server(
tmp_path, monkeypatch, pooled, operation
):
# Only redirect the socket namespace; all terminal/tmux operations are real.
with tempfile.TemporaryDirectory(prefix="oh-tmux-") as socket_dir:
monkeypatch.setenv("TMUX_TMPDIR", socket_dir)
first = TmuxPanePool(str(tmp_path)) if pooled else TmuxTerminal(str(tmp_path))
second = TmuxPanePool(str(tmp_path)) if pooled else TmuxTerminal(str(tmp_path))
try:
first.initialize()
stale = first.checkout() if isinstance(first, TmuxPanePool) else first
stale.server.cmd("kill-server")
second.initialize()
current = second.checkout() if isinstance(second, TmuxPanePool) else second
current.send_keys("printf 'SECOND_%s\\n' CONVERSATION_MARKER")
deadline = time.monotonic() + 5
while "SECOND_CONVERSATION_MARKER" not in current.read_screen():
assert time.monotonic() < deadline
time.sleep(0.05)

if operation == "read":
assert "SECOND_CONVERSATION_MARKER" not in stale.read_screen()
elif operation == "write":
marker = tmp_path / "wrong-conversation"
stale.send_keys(f"touch {marker}")
current.send_keys("printf 'WRITE_%s\\n' BARRIER")
deadline = time.monotonic() + 5
while "WRITE_BARRIER" not in current.read_screen():
assert time.monotonic() < deadline
time.sleep(0.05)
assert not marker.exists()
else:
first.close()
assert "SECOND_CONVERSATION_MARKER" in current.read_screen()
finally:
first.close()
second.close()
Loading