Skip to content

Conversation

@neubig
Copy link

@neubig neubig commented Feb 5, 2026

Fixes: #624

Previously, new_session() would:

  1. Run 'tmux new-session -P -F#{session_id}' to create session
  2. Immediately run 'tmux list-sessions' to fetch full session data

This created a race condition in some environments where list-sessions might not see the newly created session yet, causing TmuxObjectDoesNotExist errors.

The fix:

  • Expand the -F format string to include all Obj fields (except 'server')
  • Parse the output directly into a Session object
  • Eliminate the separate list-sessions query entirely

This is more efficient (one fewer subprocess call) and eliminates the race condition by making session creation atomic.

I tested this using the script in the original issue and it works in all three cases:

Environment Result
Python 3.12 + PyInstaller + Docker PASS ✓
Python 3.13 + PyInstaller + Docker PASS ✓
Python 3.13 + PyInstaller + Native PASS ✓

@CLAassistant
Copy link

CLAassistant commented Feb 5, 2026

CLA assistant check
All committers have signed the CLA.

@neubig neubig force-pushed the fix/new-session-race-condition branch from 0f803dd to e81a44f Compare February 5, 2026 05:46
@tony
Copy link
Member

tony commented Feb 5, 2026

@neubig Thank you. Can you sign the CLA?

Is there a CI or full traceback for this? Any tmux version you're trying with? Thanks for the details in #624.

@tony tony self-requested a review February 5, 2026 23:19
@neubig
Copy link
Author

neubig commented Feb 6, 2026

Thanks @tony! Signed the CLA.

I checked and the version is tmux 3.5a.

And in terms of CI where this failed, the reason why we were able to identify this is because this CI run failed: https://github.com/OpenHands/software-agent-sdk/actions/runs/21638051841/job/62369267088

We were able to trace it back to the upgrade from Python 3.12 to 3.13, and after reverting to 3.12, the consistent failures due to the issue I cited here disappeared (although this particular run still has a few failures): https://github.com/OpenHands/software-agent-sdk/actions/runs/21710957108

@neubig neubig requested a review from tony February 6, 2026 13:42
openhands-agent and others added 3 commits February 8, 2026 18:37
Previously, new_session() would run 'tmux new-session -P -F#{session_id}'
then immediately query 'tmux list-sessions' to fetch full session data.
This created a race condition in PyInstaller + Python 3.13+ + Docker
environments where list-sessions might not see the newly created session.

The fix expands the -F format string to include all Obj fields, parsing
the output directly into a Session object without a separate query.

Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: Tony Narlock <tony@git-pull.com>
Co-authored-by: Tony Narlock <tony@git-pull.com>
@tony tony force-pushed the fix/new-session-race-condition branch from 8b00212 to 058ee6d Compare February 9, 2026 00:37
@codecov
Copy link

codecov bot commented Feb 9, 2026

Codecov Report

❌ Patch coverage is 80.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.43%. Comparing base (9da19ba) to head (bc33be0).

Files with missing lines Patch % Lines
src/libtmux/neo.py 75.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #625      +/-   ##
==========================================
+ Coverage   45.39%   45.43%   +0.03%     
==========================================
  Files          22       22              
  Lines        2249     2256       +7     
  Branches      360      360              
==========================================
+ Hits         1021     1025       +4     
- Misses       1082     1085       +3     
  Partials      146      146              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tony
Copy link
Member

tony commented Feb 9, 2026

@neubig Thank you very much! I'll look at merging this as a pre-release tomorrow so we can test if it works on your side.

@neubig
Copy link
Author

neubig commented Feb 9, 2026

Great, thank you!

why: Graham's race-condition fix introduced get_output_format() and
parse_output() in neo.py; fetch_objs() had duplicate logic doing the
same thing. Functions also lacked the NumPy-style docstrings and doctests
required by project standards.
what:
- Add NumPy-style docstrings with doctests to get_output_format(),
  parse_output(), and fetch_objs()
- Make parse_output() call get_output_format() instead of duplicating
  the field-list computation
- Refactor fetch_objs() to use get_output_format() and parse_output()
  instead of inline format/parse logic
- Cache get_output_format() with @functools.cache (fields are static)
- Remove unused formats import from server.py
- Simplify format_string = get_output_format()[1] in new_session()
@tony tony force-pushed the fix/new-session-race-condition branch from e672929 to bc33be0 Compare February 9, 2026 18:48
why: The race condition fix changed new_session() to construct Session
from -P output instead of a follow-up list-sessions query, but had no
dedicated test verifying session_id and session_name are populated.
what:
- Add test asserting session_id is not None and session_name matches
@tony tony force-pushed the fix/new-session-race-condition branch from bc33be0 to 6d202e0 Compare February 9, 2026 18:50
@tony
Copy link
Member

tony commented Feb 9, 2026

@neubig (when github is back online): what's the best way for you to test this? Can you pin the branch? Do you need a PyPI release (and would an alpha do?)

Any disagreement / opinion on the changes I made in the PR?

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.

Race condition in new_session() in some environments (e.g. PyInstaller + Python 3.13+ + Docker)

4 participants