Fix connection handling for WSL2 and improve reliability#78
Open
isaacrowntree wants to merge 1 commit intoahujasid:mainfrom
Open
Fix connection handling for WSL2 and improve reliability#78isaacrowntree wants to merge 1 commit intoahujasid:mainfrom
isaacrowntree wants to merge 1 commit intoahujasid:mainfrom
Conversation
- Bind Remote Script socket to 0.0.0.0 instead of localhost, allowing connections from WSL2 and other network interfaces - Add ABLETON_HOST and ABLETON_PORT env vars for configurable connection target (defaults to localhost:9877) - Fix stale connection detection: replace sendall(b'') with MSG_PEEK-based liveness check that actually detects dead sockets - Add connect timeout (5s) to prevent hanging on unreachable hosts - Remove redundant validation command during connection establishment - Include host:port in error messages for easier debugging Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review Summary by QodoFix WSL2 connection handling and improve socket reliability
WalkthroughsDescription• Bind Remote Script to 0.0.0.0 for WSL2 and cross-network connectivity • Add ABLETON_HOST/ABLETON_PORT environment variables for flexible configuration • Replace unreliable sendall(b'') with MSG_PEEK-based socket liveness detection • Add 5-second connection timeout to prevent hanging on unreachable hosts • Remove redundant validation command during connection establishment • Improve error messages with host:port information for debugging Diagramflowchart LR
A["Remote Script<br/>Binding"] -->|"Changed to<br/>0.0.0.0"| B["Accept WSL2<br/>Connections"]
C["Environment<br/>Variables"] -->|"ABLETON_HOST<br/>ABLETON_PORT"| D["Configurable<br/>Connection Target"]
E["Socket Liveness<br/>Check"] -->|"MSG_PEEK<br/>instead of sendall"| F["Reliable Dead<br/>Socket Detection"]
G["Connection<br/>Establishment"] -->|"Add 5s<br/>timeout"| H["Prevent Hanging<br/>on Unreachable Host"]
I["Connection<br/>Validation"] -->|"Remove redundant<br/>command"| J["Faster Connection<br/>Setup"]
File Changes1. AbletonMCP_Remote_Script/__init__.py
|
Code Review by Qodo
1. Env port parse crash
|
📝 WalkthroughWalkthroughThe HOST constant is changed to listen on all network interfaces (0.0.0.0). The Ableton connection logic is refactored to use environment-based host/port configuration, implement liveness checking via socket probing, establish connections with timeout management, and improve error logging with connection context. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can disable sequence diagrams in the walkthrough.Disable the |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+11
to
+12
| ABLETON_HOST = os.environ.get("ABLETON_HOST", "localhost") | ||
| ABLETON_PORT = int(os.environ.get("ABLETON_PORT", "9877")) |
There was a problem hiding this comment.
1. Env port parse crash 🐞 Bug ⛯ Reliability
MCP_Server/server.py parses ABLETON_PORT with int() at import time; any non-integer value raises ValueError and prevents the MCP server module from loading. This turns a simple misconfiguration into a hard startup failure.
Agent Prompt
### Issue description
`ABLETON_PORT = int(os.environ.get(...))` runs at import time and can raise `ValueError`, preventing the MCP server from starting when the environment variable is misconfigured.
### Issue Context
This module is used as the package entrypoint (`ableton-mcp = "MCP_Server.server:main"`) and via `python -m MCP_Server.server`, so import-time failures are fatal.
### Fix Focus Areas
- MCP_Server/server.py[11-13]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| # Constants for socket communication | ||
| DEFAULT_PORT = 9877 | ||
| HOST = "localhost" | ||
| HOST = "0.0.0.0" |
There was a problem hiding this comment.
2. Unauthenticated remote control 🐞 Bug ⛨ Security
AbletonMCP_Remote_Script now binds the command socket to 0.0.0.0, exposing it on all network interfaces while still accepting and executing JSON commands without authentication. Any host that can reach port 9877 can control Ableton Live over the network.
Agent Prompt
### Issue description
Binding the Ableton Remote Script to `0.0.0.0` exposes the control socket on all interfaces, but the protocol has no authentication, so any reachable host can send commands.
### Issue Context
The script accepts TCP connections and executes JSON commands directly, so network exposure materially changes the threat model.
### Fix Focus Areas
- AbletonMCP_Remote_Script/__init__.py[17-21]
- AbletonMCP_Remote_Script/__init__.py[75-82]
- AbletonMCP_Remote_Script/__init__.py[92-112]
- AbletonMCP_Remote_Script/__init__.py[133-167]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
MCP_Server/server.py (2)
11-12: Consider handling invalidABLETON_PORTvalues gracefully.If
ABLETON_PORTis set to a non-numeric value,int()will raise aValueErrorat module load time with an unclear error message. Consider wrapping this in a try-except or validating the port.💡 Proposed defensive handling
ABLETON_HOST = os.environ.get("ABLETON_HOST", "localhost") -ABLETON_PORT = int(os.environ.get("ABLETON_PORT", "9877")) +try: + ABLETON_PORT = int(os.environ.get("ABLETON_PORT", "9877")) +except ValueError: + logger.warning("Invalid ABLETON_PORT value, using default 9877") + ABLETON_PORT = 9877Note: This requires moving the logger initialization before these lines, or using a print statement for the warning.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCP_Server/server.py` around lines 11 - 12, ABLETON_PORT is converted with int(...) at import time which will raise ValueError on non-numeric env values; update the module to validate/parse ABLETON_PORT with a try-except (or use str.isdigit() check) and fall back to the default 9877 on failure, emitting a warning via the module logger (so move logger initialization above these constants or use print if not possible). Reference the ABLETON_PORT and ABLETON_HOST symbols when making the change and ensure the code does not raise at module load but instead logs the invalid value and continues with the default port.
205-218: MSG_PEEK liveness check is sound, but consider catchingOSErrorfor robustness.The MSG_PEEK approach correctly detects dead connections. However, catching only
BlockingIOErrormay miss some error conditions. On certain platforms or Python versions, non-blocking socket operations can raiseOSErrorwitherrno.EAGAINorerrno.EWOULDBLOCK.💡 Broader exception handling for cross-platform safety
_ableton_connection.sock.setblocking(False) try: data = _ableton_connection.sock.recv(1, socket.MSG_PEEK) if data == b'': raise ConnectionError("Remote end closed") - except BlockingIOError: - pass # Socket is alive, just no data waiting — this is normal + except (BlockingIOError, OSError) as e: + # BlockingIOError or EAGAIN/EWOULDBLOCK means socket is alive + import errno + if isinstance(e, OSError) and e.errno not in (errno.EAGAIN, errno.EWOULDBLOCK): + raise + # Socket is alive, just no data waiting — this is normal finally: _ableton_connection.sock.setblocking(True)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCP_Server/server.py` around lines 205 - 218, The MSG_PEEK liveness check currently only catches BlockingIOError; update the inner recv exception handling for _ableton_connection.sock.recv to also catch OSError and treat errno.EAGAIN / errno.EWOULDBLOCK the same as BlockingIOError (i.e., ignore as "no data yet"), while re-raising or converting other OSErrors into connection failures; reference the symbols _ableton_connection, sock, recv, BlockingIOError, OSError, and errno.EAGAIN/errno.EWOULDBLOCK when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@MCP_Server/server.py`:
- Around line 11-12: ABLETON_PORT is converted with int(...) at import time
which will raise ValueError on non-numeric env values; update the module to
validate/parse ABLETON_PORT with a try-except (or use str.isdigit() check) and
fall back to the default 9877 on failure, emitting a warning via the module
logger (so move logger initialization above these constants or use print if not
possible). Reference the ABLETON_PORT and ABLETON_HOST symbols when making the
change and ensure the code does not raise at module load but instead logs the
invalid value and continues with the default port.
- Around line 205-218: The MSG_PEEK liveness check currently only catches
BlockingIOError; update the inner recv exception handling for
_ableton_connection.sock.recv to also catch OSError and treat errno.EAGAIN /
errno.EWOULDBLOCK the same as BlockingIOError (i.e., ignore as "no data yet"),
while re-raising or converting other OSErrors into connection failures;
reference the symbols _ableton_connection, sock, recv, BlockingIOError, OSError,
and errno.EAGAIN/errno.EWOULDBLOCK when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f64f02b0-fa85-43d6-a256-12f22b74b18d
📒 Files selected for processing (2)
AbletonMCP_Remote_Script/__init__.pyMCP_Server/server.py
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
1 participant
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.
Summary
Fixes for running the MCP server from WSL2 connecting to Ableton Live on Windows, and general connection reliability improvements.
0.0.0.0instead oflocalhostso it accepts connections from WSL2 and other network interfacesABLETON_HOST/ABLETON_PORTenv vars for configurable connection target (defaults tolocalhost:9877) — useful for WSL2, Docker, or remote setupssendall(b'')check never actually detected dead sockets; replaced withMSG_PEEK-based liveness checkContext
When running Claude Code's MCP client in WSL2 on Windows 11, the MCP server could not connect to Ableton's Remote Script because:
localhost(loopback), unreachable from WSL2's network namespacesendall(b'')) silently succeeded on dead sockets, causing stale connections to never recoverTest plan
get_session_info,create_clip,add_notes_to_clip,fire_clipall worklocalhostpreserved)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Improvements