Repository navigation
fix(agent-server): keep tmux socket paths within the Unix socket limit - #5559
Merged
Merged
Conversation
Since terminals got unique socket names (openhands-<32 hex>, #5485), any TMUX_TMPDIR longer than about 54 bytes on Linux (about 51 on macOS) makes every terminal fail with "LibTmuxException: new-session: error connecting to ... (File name too long)": <dir>/tmux-<uid>/<name> no longer fits sun_path (108 bytes on Linux, 104 on macOS, NUL included). Agent Canvas hits this in CI, where TMUX_TMPDIR sits under the checkout's .tmp state dir, and the server's own default under macOS's per-user $TMPDIR is too deep as well. The agent-server now falls back to a per-server directory under /tmp when the configured or default TMUX_TMPDIR cannot hold a socket path, warns when it overrides an explicit value, and restores that value on shutdown. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DdnhYnKDzEpfWiZNr8kDsa
Contributor
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
Contributor
Coverage Report •
|
||||||||||||||||||||
enyst
marked this pull request as ready for review
October 8, 2026 13:05
all-hands-bot
approved these changes
Oct 8, 2026
all-hands-bot
left a comment
all-hands-bot
left a comment
Collaborator
There was a problem hiding this comment.
Approved ✅
Reviewed the diff (openhands-agent-server/.../api.py, tests/agent_server/test_api.py) at head 55f5488, linked issue #5558 and its acceptance criteria, and current-head CI (all green, including agent-server-tests, workspace-tests, windows-tests).
What I checked and confirmed:
- Socket-path math is right.
sockaddr_un.sun_pathis 108 bytes on Linux and 104 on macOS including the NUL, so the 107/103 maxima are correct. I reproduced the boundary against real tmux on Linux: a 107-byte$TMUX_TMPDIR/tmux-<uid>/openhands-<32hex>path starts a server; 108 fails withFile name too long. os.path.realpath()in_fits_tmux_socketis load-bearing, not incidental. tmux resolves$TMUX_TMPDIRbefore binding — I verified a short symlink pointing at a deep target fails, while a deep symlink pointing at a short target works. Using the resolved path both rejects the first case (correctly) and accepts the second (correctly); without it, the first would be accepted and break. Nice detail.- The
/tmpfallback fits. On macOS/tmpresolves to/private/tmp, giving ~87 bytes for the socket path, under 103; the stock macOS$TMPDIR(~131 bytes) is correctly detected as too deep. - The restore change fixes a real bug. Reinstating
original_tmux_tmpdirinstead of unconditionally popping means an explicit-but-too-deep value is no longer left unset after shutdown. - Acceptance criteria covered. Deep explicit
TMUX_TMPDIR→ overridden with a warning and a working pool (the new test starts a realTmuxPanePool); unset + deep temp dir → fallback fits; short explicit value → honored unchanged. The per-pool socket naming and #5485 isolation are untouched. - Evidence. The PR body shows the original symptom on the base revision, the same workflow passing on this head, plus the real OpenHands live E2E spec through Agent Canvas with logs, and explicitly flags that macOS was computed rather than reproduced. That matches what I'd expect for a runtime-lifecycle bug fix.
Non-blocking observations (no action needed for approval):
- The
/tmpfallback is not re-validated with_fits_tmux_socket; it happens to fit on both platforms today, so it's fine, but it is a hardcoded assumption. - Repo memory in
AGENTS.mdstill states the server "respects an explicitTMUX_TMPDIRoverride" unconditionally; a one-line update noting the depth guard would keep it accurate.
This review was created by an AI agent (OpenHands) on behalf of the requesting user.
This was referenced Oct 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HUMAN:
I, Engel Nyst (@enyst), asked an AI agent to help with project maintenance work.
AGENT:
I'm an AI agent based on Opus 5.5, helping Engel Nyst (@enyst) with project maintenance.
Why
Since #5485, each terminal pool gets its own tmux socket,
openhands-<32 hex>. That makes the socket path 33 bytes longer, so aTMUX_TMPDIRdeeper than about 54 bytes (Linux) or 51 bytes (macOS) no longer fitssun_path. Every terminal then fails withLibTmuxException ... (File name too long), and the conversation's first message returns 500. Agent Canvas's live E2E job has failed this way on every run since it pinned 1.53.0 (OpenHands/OpenHands#18082). The server's own default under macOS's$TMPDIRis over the limit too.Summary
$TMUX_TMPDIR/tmux-<uid>/openhands-<32 hex>fits the platform's socket path limit. If the configured or default directory is too deep, it uses/tmp/openhands-agent-server-<pid>instead.TMUX_TMPDIR, and restores the original value on shutdown.TMUX_TMPDIRis honored exactly as before. Socket naming and the isolation from fix(terminal): isolate tmux sockets to prevent cross-conversation handle reuse #5485 are unchanged.Issue Number
Fixes #5558
How to Test
Unit tests:
The new
test_ensure_server_tmux_tmpdir_replaces_too_deep_envstarts a realTmuxPanePoolunder a too-deepTMUX_TMPDIR. The existing tmux-dir tests now use a short temp dir, because pytest'stmp_pathis itself too deep for a socket, which is the same trap #5494 hit.Before and after, with a 64-byte-deep
TMUX_TMPDIR:End to end through Agent Canvas, the real downstream workflow:
/home/runner/work/OpenHands/OpenHands.TMUX_TMPDIRwas the stock.tmp/live-e2e-state/tmux, andOH_AGENT_SERVER_LOCAL_PATHpointed at this branch.On 1.53.0 that spec fails with
File name too long. On this branch the server logs the following, and the spec passes:I did not run this on macOS. The macOS numbers are computed from the 104-byte
sun_path.Video/Screenshots
N/A (server-side change; logs above).
Design Doc
N/A
Type
Notes
TMUX_TMPDIRso it can go green before a release with this fix. This PR is the general fix for every deployment./var/folders/…/T/openhands-agent-server-<pid>on macOS is too deep even for a 12-hex suffix.🤖 Generated with Claude Code
https://claude.ai/code/session_01DdnhYnKDzEpfWiZNr8kDsa
Generated by Claude Code
🐳 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
eclipse-temurin:17-jdkpython-node-runtimepython-node-runtimepython-node-runtimegolang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:55f5488-pythonRun
All tags pushed for this build
About Multi-Architecture Support
55f5488-python) is a multi-arch manifest supporting both amd64 and arm6455f5488-python-amd64) are also available if needed