Skip to content

test(agent-server): avoid overlong tmux test socket paths on macOS - #5619

Merged
enyst merged 1 commit into
OpenHands:mainfrom
enyst:fix/tmux-test-socket-path-macos
Oct 9, 2026
Merged

enyst merged 1 commit into
OpenHands:mainfrom
enyst:fix/tmux-test-socket-path-macos

Conversation

@enyst

@enyst enyst commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

HUMAN:
This PR fixes a little annoyance in tests, which sometimes end up creating a too long path on MacOS.

⸻

AGENT:

I’m an AI agent (GPT-6) helping Engel Nyst (@enyst) with project maintenance.

Why

Follow-up to #5559, which fixed tmux Unix socket path limits in the Agent Server.

The production fix is correct, but several tests construct their expected TMUX_TMPDIR beneath the randomized short_tmp_path fixture and then append openhands-agent-server-.

On macOS, /tmp resolves to /private/tmp. With UID 501 and PID 12345, the resulting socket path can reach 105 bytes, exceeding the 103-byte usable limit. The production fallback under /tmp, by contrast, needs only 93 bytes.

Consequently, the tests can fail on macOS even though the production implementation behaves correctly.

The tests should not accidentally recreate the exact failure the production code is designed to avoid.

Summary

  • Separate default-path selection tests from directory-creation tests, avoiding randomized fixture paths in assertions about the production default.
  • Use a genuinely short final directory for the real TmuxPanePool integration test and the lifespan test.
  • Add deterministic macOS/Linux socket-path byte-length tests using simulated canonical paths, without globally monkeypatching process identity.

No production code changes. The per-process directory name, socket isolation, path-length validation, and /tmp fallback are unchanged.

Follow-up to #5559 (original bug #5558).

Issue Number

Fixes #5620

How to Test

The change is limited to tests/agent_server/test_api.py.

Review verification:

  • Confirmed the fork branch is one commit ahead of upstream and changes only the test file.
  • Verified the modeled socket-path lengths: 105 bytes for the old fixture-derived path versus 93 bytes for the production fallback.
  • No local pytest run was performed in this Chat environment. CI results are not yet available.

Run the relevant tests:

uv run pytest tests/agent_server/test_api.py -n 4

The existing tmux tests require tmux on a POSIX host.

The new parameterized test simulates the canonical path and socket limits for both macOS and Linux, so the boundary behavior can also be checked by Linux CI.

Expected behavior:

  • Short default path: selected correctly.
  • Deep default path: /tmp fallback selected.
  • Explicit deep TMUX_TMPDIR: replaced, and a real tmux pool initializes.
  • Directory creation and lifespan restoration: continue to work.
  • Existing short explicit TMUX_TMPDIR: remains honored.

Video/Screenshots

N/A. Test-only change; reproduction details are included above.

Design Doc

N/A. No production design or API changes.

Type

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

Notes

Commit: 32cfd08

Branch: enyst:fix/tmux-test-socket-path-macos

@all-hands-bot all-hands-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. Test-only change that correctly stops the tests from recreating the macOS socket-length failure the production code guards against.

Verified locally on tests/agent_server/test_api.py:

  • Full file: uv run pytest tests/agent_server/test_api.py -> 47 passed (tmux subset -k tmux -> 9 passed).
  • New test_default_server_tmux_tmpdir_counts_canonical_socket_bytes[macos] and [linux] both pass on Linux CI, confirming the 103/107 boundary is exercised from one platform.
  • ruff check and ruff format --check clean.

Design reads well: separating default-path selection from directory creation, using a genuinely short final dir for the real TmuxPanePool/lifespan tests, and keeping the /tmp fallback deterministic. No production changes, as advertised.

Two minor, non-blocking notes:

  1. The new parameterized test hardcodes the 103/107 limits, duplicating _MAX_TMUX_SOCKET_PATH in api.py. A short comment pointing at that constant would help if the limits ever change.
  2. test_default_server_tmux_tmpdir_uses_current_pid no longer carries the posix skipif (its sibling still does). It stays safe because _fits_tmux_socket returns True on non-posix, but re-adding the marker would keep intent explicit.

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

@enyst
enyst merged commit d1bfda2 into OpenHands:main Oct 9, 2026
45 of 49 checks passed
@enyst
enyst deleted the fix/tmux-test-socket-path-macos branch October 9, 2026 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Agent Server tmux test fixtures exceed macOS Unix socket path limit

2 participants