fix(terminal): isolate tmux sockets to prevent cross-conversation handle reuse

[neubig]

HUMAN:

I confirmed the bug before and ran the code after with no issue.

---

AGENT:
This PR was created by an AI agent (OpenHands) on behalf of the user.

Why

Terminal tools share a tmux socket. After server death/restart, numeric pane/window/session IDs are reused; retained libtmux handles can read, write to, or kill another conversation. Local saved observations showed a sleep command returning a different workspace and later another shell command. No private conversation data is included here.

Summary

• Allocate a fresh UUID socket for each pooled or standalone terminal owner, including recovery generations.
• Preserve startup cleanup for legacy sockets and UUID sockets within the configured tmux directory; leave unrelated names alone.
• Add six real-tmux read/write/close isolation regressions plus real startup-cleanup coverage.

Issue Number

Fixes #5484

How to Test

Requires tmux and the repository development dependencies.

uv run pytest tests/tools/terminal/test_tmux_pane_pool.py tests/agent_server/test_api.py

Result: 72 passed. These are real shell/tmux integration paths, not mocked terminal operations. Tests use a temporary TMUX_TMPDIR for server destruction. Six isolation cases fail against the original terminal source; all pass after this change. The cleanup test also failed before its implementation update and passes afterward.

Full run: uv run pytest tests/tools/terminal tests/agent_server/test_api.py -> 379 passed, 15 failed, 15 skipped. Existing concurrency/timeout/prompt tests failed in this local environment; a clean upstream worktree also failed concurrency and tmux timeout tests (2 failed, 9 passed on the executor/exit-code subset). Not every full-suite failure has been proven pre-existing. Keep draft pending CI/investigation.

Ruff format/lint, pycodestyle, pyright, dynamic-attribute, import-rule, and tool-registration hooks pass when invoked individually. The all-hooks invocation was blocked by a broken local virtualenv pip seed cache while initializing the YAML formatter; no YAML changed. Existing compatible development dependencies were reused via UV_PROJECT_ENVIRONMENT and PYTHONPATH.

Video/Screenshots

Not applicable: backend change. Reproduction uses real tmux restart, output marker, command-created file, and stale-owner close assertions.

Design Doc

Each terminal owner gets openhands-<uuid hex> rather than the global openhands socket. Panes within one pool still share that pool server. This removes cross-owner numeric-ID reuse without racy ownership check-then-use logic. Trade-off: one tmux server process per owner rather than one shared process. Normal close kills the sole session; startup cleanup recognizes both formats.

This is accidental session isolation, not a security boundary between same-UID processes. Startup cleanup retains its existing assumption that the configured TMUX_TMPDIR is owned by the Agent Server; independently running servers should not deliberately share it.

Type

• Bug fix
• Feature
• Refactor
• Breaking change
• Docs / chore

Notes

The running local Agent Server was not modified or restarted. A release/dependency update and runtime restart are needed to deploy the fix. The human contributor must fill the HUMAN section before marking ready.

---

🐳 Agent Server images for this PR — GHCR package, pull/run commands, and all pushed tags (click to expand)

• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server

Variants & Base Images

Variant	Architectures	Base Image	Docs / Tags
java	amd64, arm64	eclipse-temurin:17-jdk	Link
python-slim	amd64, arm64	python-node-runtime	Link
python-minimal	amd64, arm64	python-node-runtime	Link
python	amd64, arm64	python-node-runtime	Link
golang	amd64, arm64	golang:1.21-bookworm	Link

Pull (multi-arch manifest)

# Each variant is a multi-arch manifest supporting both amd64 and arm64
docker pull ghcr.io/openhands/agent-server:8972e85-python

Run

docker run -it --rm \
  -p 8000:8000 \
  --name agent-server-8972e85-python \
  ghcr.io/openhands/agent-server:8972e85-python

All tags pushed for this build

ghcr.io/openhands/agent-server:8972e85-golang-amd64
ghcr.io/openhands/agent-server:8972e85ba51d192c92eb1a64f4a20307256ca143-golang-amd64
ghcr.io/openhands/agent-server:fix-terminal-tmux-server-isolation-golang-amd64
ghcr.io/openhands/agent-server:8972e85-golang_tag_1.21-bookworm-amd64
ghcr.io/openhands/agent-server:8972e85-golang-arm64
ghcr.io/openhands/agent-server:8972e85ba51d192c92eb1a64f4a20307256ca143-golang-arm64
ghcr.io/openhands/agent-server:fix-terminal-tmux-server-isolation-golang-arm64
ghcr.io/openhands/agent-server:8972e85-golang_tag_1.21-bookworm-arm64
ghcr.io/openhands/agent-server:8972e85-java-amd64
ghcr.io/openhands/agent-server:8972e85ba51d192c92eb1a64f4a20307256ca143-java-amd64
ghcr.io/openhands/agent-server:fix-terminal-tmux-server-isolation-java-amd64
ghcr.io/openhands/agent-server:8972e85-eclipse-temurin_tag_17-jdk-amd64
ghcr.io/openhands/agent-server:8972e85-java-arm64
ghcr.io/openhands/agent-server:8972e85ba51d192c92eb1a64f4a20307256ca143-java-arm64
ghcr.io/openhands/agent-server:fix-terminal-tmux-server-isolation-java-arm64
ghcr.io/openhands/agent-server:8972e85-eclipse-temurin_tag_17-jdk-arm64
ghcr.io/openhands/agent-server:8972e85-python-amd64
ghcr.io/openhands/agent-server:8972e85ba51d192c92eb1a64f4a20307256ca143-python-amd64
ghcr.io/openhands/agent-server:fix-terminal-tmux-server-isolation-python-amd64
ghcr.io/openhands/agent-server:8972e85-python-node-runtime-amd64
ghcr.io/openhands/agent-server:8972e85-python-arm64
ghcr.io/openhands/agent-server:8972e85ba51d192c92eb1a64f4a20307256ca143-python-arm64
ghcr.io/openhands/agent-server:fix-terminal-tmux-server-isolation-python-arm64
ghcr.io/openhands/agent-server:8972e85-python-node-runtime-arm64
ghcr.io/openhands/agent-server:8972e85-python-minimal-amd64
ghcr.io/openhands/agent-server:8972e85ba51d192c92eb1a64f4a20307256ca143-python-minimal-amd64
ghcr.io/openhands/agent-server:fix-terminal-tmux-server-isolation-python-minimal-amd64
ghcr.io/openhands/agent-server:8972e85-python-node-runtime-minimal-amd64
ghcr.io/openhands/agent-server:8972e85-python-minimal-arm64
ghcr.io/openhands/agent-server:8972e85ba51d192c92eb1a64f4a20307256ca143-python-minimal-arm64
ghcr.io/openhands/agent-server:fix-terminal-tmux-server-isolation-python-minimal-arm64
ghcr.io/openhands/agent-server:8972e85-python-node-runtime-minimal-arm64
ghcr.io/openhands/agent-server:8972e85-python-slim-amd64
ghcr.io/openhands/agent-server:8972e85ba51d192c92eb1a64f4a20307256ca143-python-slim-amd64
ghcr.io/openhands/agent-server:fix-terminal-tmux-server-isolation-python-slim-amd64
ghcr.io/openhands/agent-server:8972e85-python-node-runtime-slim-amd64
ghcr.io/openhands/agent-server:8972e85-python-slim-arm64
ghcr.io/openhands/agent-server:8972e85ba51d192c92eb1a64f4a20307256ca143-python-slim-arm64
ghcr.io/openhands/agent-server:fix-terminal-tmux-server-isolation-python-slim-arm64
ghcr.io/openhands/agent-server:8972e85-python-node-runtime-slim-arm64
ghcr.io/openhands/agent-server:8972e85-golang
ghcr.io/openhands/agent-server:8972e85ba51d192c92eb1a64f4a20307256ca143-golang
ghcr.io/openhands/agent-server:fix-terminal-tmux-server-isolation-golang
ghcr.io/openhands/agent-server:8972e85-golang_tag_1.21-bookworm
ghcr.io/openhands/agent-server:8972e85-java
ghcr.io/openhands/agent-server:8972e85ba51d192c92eb1a64f4a20307256ca143-java
ghcr.io/openhands/agent-server:fix-terminal-tmux-server-isolation-java
ghcr.io/openhands/agent-server:8972e85-eclipse-temurin_tag_17-jdk
ghcr.io/openhands/agent-server:8972e85-python-minimal
ghcr.io/openhands/agent-server:8972e85ba51d192c92eb1a64f4a20307256ca143-python-minimal
ghcr.io/openhands/agent-server:fix-terminal-tmux-server-isolation-python-minimal
ghcr.io/openhands/agent-server:8972e85-python-node-runtime-minimal
ghcr.io/openhands/agent-server:8972e85-python-slim
ghcr.io/openhands/agent-server:8972e85ba51d192c92eb1a64f4a20307256ca143-python-slim
ghcr.io/openhands/agent-server:fix-terminal-tmux-server-isolation-python-slim
ghcr.io/openhands/agent-server:8972e85-python-node-runtime-slim
ghcr.io/openhands/agent-server:8972e85-python
ghcr.io/openhands/agent-server:8972e85ba51d192c92eb1a64f4a20307256ca143-python
ghcr.io/openhands/agent-server:fix-terminal-tmux-server-isolation-python
ghcr.io/openhands/agent-server:8972e85-python-node-runtime

About Multi-Architecture Support

• Each variant tag (e.g., 8972e85-python) is a multi-arch manifest supporting both amd64 and arm64
• Docker automatically pulls the correct architecture for your platform
• Individual architecture tags (e.g., 8972e85-python-amd64) are also available if needed

[github-actions]

  ✅ **PR Artifacts Cleaned Up**

  The `.pr/` directory is no longer present.

[github-actions]

REST API breakage checks (OpenAPI) — ✅ PASSED

Result: ✅ PASSED

Action log

[github-actions]

Coverage Report •

File	Stmts	Miss	Cover	Missing
openhands-agent-server/openhands/agent_server
api.py	333	22	93%	138, 152–155, 208, 222, 228, 284, 289, 302–304, 340, 346, 350, 371–372, 565, 631, 634, 640
openhands-tools/openhands/tools/terminal/terminal
tmux_pane_pool.py	144	29	80%	51–53, 111, 118, 166–167, 175, 229, 233–234, 254–255, 271, 274–275, 277–280, 283–284, 288–292, 294–295
tmux_terminal.py	126	71	44%	34–38, 42–43, 47–50, 117–118, 120, 123–124, 126, 129–130, 132, 134, 136–138, 145–146, 150–151, 154–155, 160–164, 168, 171, 173–175, 179–184, 188–189, 206, 213–214, 219–220, 235, 257, 269–276, 284–285, 287–288, 290, 292–294
TOTAL	47959	19582	59%

[neubig]

@all-hands-bot Please review this terminal isolation fix, including stale-handle safety, lifecycle cleanup, and the real-tmux regression coverage.

This review request was posted by an AI agent (OpenHands) on behalf of the user.

[supramaniam5637-cmd]

HUMAN:

I confirmed the bug before and ran the code after with no issue.

AGENT: This PR was created by an AI agent (OpenHands) on behalf of the user.

Why

Terminal tools share a tmux socket. After server death/restart, numeric pane/window/session IDs are reused; retained libtmux handles can read, write to, or kill another conversation. Local saved observations showed a sleep command returning a different workspace and later another shell command. No private conversation data is included here.

Summary

• Allocate a fresh UUID socket for each pooled or standalone terminal owner, including recovery generations.
• Preserve startup cleanup for legacy sockets and UUID sockets within the configured tmux directory; leave unrelated names alone.
• Add six real-tmux read/write/close isolation regressions plus real startup-cleanup coverage.

Issue Number

Fixes #5484

How to Test

Requires tmux and the repository development dependencies.

uv run pytest tests/tools/terminal/test_tmux_pane_pool.py tests/agent_server/test_api.py

Result: 72 passed. These are real shell/tmux integration paths, not mocked terminal operations. Tests use a temporary TMUX_TMPDIR for server destruction. Six isolation cases fail against the original terminal source; all pass after this change. The cleanup test also failed before its implementation update and passes afterward.

Full run: uv run pytest tests/tools/terminal tests/agent_server/test_api.py -> 379 passed, 15 failed, 15 skipped. Existing concurrency/timeout/prompt tests failed in this local environment; a clean upstream worktree also failed concurrency and tmux timeout tests (2 failed, 9 passed on the executor/exit-code subset). Not every full-suite failure has been proven pre-existing. Keep draft pending CI/investigation.

Ruff format/lint, pycodestyle, pyright, dynamic-attribute, import-rule, and tool-registration hooks pass when invoked individually. The all-hooks invocation was blocked by a broken local virtualenv pip seed cache while initializing the YAML formatter; no YAML changed. Existing compatible development dependencies were reused via UV_PROJECT_ENVIRONMENT and PYTHONPATH.

Video/Screenshots

Not applicable: backend change. Reproduction uses real tmux restart, output marker, command-created file, and stale-owner close assertions.

Design Doc

Each terminal owner gets openhands-<uuid hex> rather than the global openhands socket. Panes within one pool still share that pool server. This removes cross-owner numeric-ID reuse without racy ownership check-then-use logic. Trade-off: one tmux server process per owner rather than one shared process. Normal close kills the sole session; startup cleanup recognizes both formats.

This is accidental session isolation, not a security boundary between same-UID processes. Startup cleanup retains its existing assumption that the configured TMUX_TMPDIR is owned by the Agent Server; independently running servers should not deliberately share it.

ssstik.io_1773373436533.mp3

Type

• Bug fix
• Feature
• Refactor
• Breaking change
• Docs / chore

Notes

The running local Agent Server was not modified or restarted. A release/dependency update and runtime restart are needed to deploy the fix. The human contributor must fill the HUMAN section before marking ready.

🐳 Agent Server images for this PR — GHCR package, pull/run commands, and all pushed tags (click to expand)

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Review summary

Scope. This change is correctly scoped to this repository: it modifies the agent-server's tmux startup cleanup (openhands-agent-server/openhands/agent_server/api.py) and the two terminal owners in openhands-tools. Linked issue #5484 is open, ready-for-dev, priority:medium, and its acceptance criteria are addressed.

Verified on the current head 8972e85 (detached checkout, no tracked files modified):

• uv run pytest tests/tools/terminal/test_tmux_pane_pool.py tests/agent_server/test_api.py → 72 passed, matching the PR's claim.
• I ran the seven new regressions against the base revision (1b2b5c8) with the same tests: all 7 fail (read/write/close × pooled/standalone, plus the cleanup test) and all pass on the head, so the new tests are meaningful before/after evidence rather than tautologies.
• Current-head check runs are green. The only non-success run, Validate PR description, failed at 21:21:38 and was superseded by a success at 21:22:19 on the same head.

Implementation assessment. Giving each terminal owner a fresh openhands-<uuid4().hex> socket removes the numeric-ID aliasing between owners, and cleanup globbing openhands- + 32 hex characters matches only the new socket format while leaving unrelated sockets alone. Recovery generations (_initialize_pool) get new sockets too. Startup cleanup only runs in the agent-server, while TmuxPanePool/TmuxTerminal are also constructed directly in SDK embeddings; I considered whether that leaks servers there, but tmux has no auto-start daemon and no code path creates a socket for cleanup to orphan, so that risk is inherent to tmux rather than introduced here.

Non-blocking design notes (no change requested):

• Socket accumulation. TmuxPanePool.initialize() / TmuxTerminal.initialize() never remove their socket files; the single session they kill is gone, but the dead socket file remains under TMUX_TMPDIR/tmux-<uid>/ until the next agent-server startup. TMUX_TMPDIR already defaults to a per-pid directory that the agent-server lifespan removes, so the default deployment is bounded; this only matters when TMUX_TMPDIR is set explicitly and the server runs long with many pools. The head's own test asserts the stale handle keeps addressing the dead UUID socket (''), which is the intended fix, not a regression.
• The cleanup path adds os.name / os.getuid / Path.is_socket that the base did not use. These are guarded (os.getuid is only reached on POSIX, and Path.is_socket is available on Windows), and CI passes on Windows and macOS, so I see no demonstrated defect; worth a maintainer glance only because the agent-server runs on those platforms.

No material correctness, security, or compatibility defect found on the current head.

✅ APPROVED

[all-hands-bot]

Review: terminal tmux socket isolation

I reviewed the diff, reproduced the isolation tests against the pre-fix source, and ran the broader suites in a tmux 3.5a environment. Summary: the core fix is correct and the regression coverage is genuine. A few lifecycle/cleanup issues are worth addressing before this leaves draft.

What I verified

• The regression tests actually catch the bug. I checked out main's tmux_terminal.py, tmux_pane_pool.py, and api.py while keeping the new tests, and all 7 new cases failed (read/write/close × pooled/standalone, plus the startup-cleanup test). They pass on the PR branch. So this is real coverage, not a test that only asserts the new behavior.
• Targeted suite: uv run pytest tests/tools/terminal/test_tmux_pane_pool.py tests/agent_server/test_api.py → 72 passed.
• Broader suites in my env: tests/tools/terminal → 351 passed, 15 skipped; tests/agent_server/test_api.py → 43 passed. The concurrency/timeout failures called out in the PR body did not reproduce for me (likely environment-specific, as noted).
• Ruff passes on all changed files.
• Root cause / approach: correct. libtmux.Server binds a socket name, and a stale handle keeps that name, so a per-owner openhands-<uuid> socket prevents a retained handle from resolving to a restarted server's reused numeric IDs. This avoids racy check-then-use ownership logic. The pool still shares one socket across its panes, and the recovery path allocates a fresh UUID per generation as claimed. I also confirmed nothing else in the repo depends on the shared openhands socket name.

Findings

1. (Medium) Each unique socket leaks a socket file; the recommended override path never reclaims it.

tmux does not remove the socket file when the last session is killed — it lingers after both kill-server and kill-session (verified with raw tmux). Since every pool, standalone terminal, and recovery generation now gets a brand-new name, each one leaves a openhands-<uuid> file behind. The only reclamation is the next server startup's _cleanup_stale_tmux_sessions.

That is fine for the default per-process TMUX_TMPDIR (openhands-agent-server-<pid>, unique per process, though note shutdown only pops the env var — api.py has no rmtree, so the directory and files persist until the OS temp cleaner runs). But the PR explicitly recommends setting TMUX_TMPDIR when independently running servers share a directory. On a stable user-set path, these files accumulate indefinitely between restarts. Over a long-lived server with many conversations/recoveries, this is unbounded inode/FD pressure.

Suggestion: unlink the socket file after killing the session in close() (and after pool close()), or clear/remove the per-owner socket in the shutdown path when the directory was defaulted.

2. (Low) Stale-handle read fails silently, so "isolated" is not the same as "detected".

After kill-server, stale.read_screen() returns "" rather than raising — libtmux's capture-pane returns empty stdout. So a stale handle cannot distinguish "the pane is empty" from "the server is gone". The pool recovers because the command path catches LibTmuxException/TmuxObjectDoesNotExist, but a stale pooled handle returned via checkin() is not validated, so it can sit in _available and only surface later. Worth documenting as the intended behavior (the write probe is a good positive test that isolation holds), but if you want stale handles to be actively rejected, a cheap liveness check at checkout/checkin would make the failure mode explicit rather than a silent empty read.

3. (Low) Cleanup glob is narrower than the invariant.

glob(f"{TMUX_SOCKET_NAME}-" + "[0-9a-f]" * 32) matches exactly 32 lowercase hex chars. That matches uuid.uuid4().hex today, so it's correct, but it silently stops matching if the naming scheme ever changes. Consider a shared helper (or a slightly broader pattern) so the producer and the cleaner can't drift.

4. (Info) Startup warning noise on the failure path.

For any leftover socket file with no running server, libtmux.Server(...).sessions raises and the new code logs Failed to cleanup tmux socket ... at warning per file. Previously the legacy path logged missing-server at debug. Combined with finding 1, startup warnings scale with the number of leaked sockets. A quick liveness check before constructing the server (or logging at debug when the error is just "no server running") would keep this quiet.

5. (Info) Test coupling.

The close case asserts the current conversation's screen still shows its marker after first.close(). That is a valid post-fix assertion, but for the standalone variant it holds largely because the second terminal's new_session(..., kill_session=True) replaced the shared default socket; it isn't independently exercising a distinct standalone-close path. Not a blocker — just be aware the pooled and standalone parametrizations overlap more than the names suggest.

Verdict

The isolation fix and its regression tests look sound; I'd merge once the socket-file lifecycle (finding 1) is addressed, since it's the one item that can degrade a long-running deployment. Findings 2–5 are optional hardening.

Generated by OpenHands AI on behalf of the user.