Skip to content

fix: Ableton Live 10 (Python 2.7) compatibility — threadless update_display architecture#103

Open
leroidubuffet wants to merge 3 commits into
ahujasid:mainfrom
leroidubuffet:live10-compatibility
Open

fix: Ableton Live 10 (Python 2.7) compatibility — threadless update_display architecture#103
leroidubuffet wants to merge 3 commits into
ahujasid:mainfrom
leroidubuffet:live10-compatibility

Conversation

@leroidubuffet

@leroidubuffet leroidubuffet commented May 31, 2026

Copy link
Copy Markdown

Problem

Ableton Live 10 uses an embedded Python 2.7 runtime where background threads suffer from severe GIL starvation — threads can wait 60–90 seconds before getting CPU time. The original implementation spawns a socket server thread that never reliably runs, causing the MCP server's 15-second timeout to fire before any response arrives.

Solution

Rewrite the Remote Script to use no background threads at all. Instead, all socket I/O and Live API calls are handled in update_display() — a hook guaranteed to be called by Live every ~100ms on its main thread.

Key changes

  • No threads: removed threading, queue, time imports entirely
  • Non-blocking sockets: select() with timeout=0 in update_display() — returns immediately if no data
  • Session cache: built on the first update_display() tick (safe: main thread), used to respond to get_session_info instantly
  • Direct Live API calls: since we're on the main thread, no schedule_message() or queue gymnastics needed
  • ~360 lines vs original ~1000+ lines

Architecture

update_display() [main thread, every 100ms]
  ├── build session cache (first tick only)
  ├── accept new connection (non-blocking select)
  └── receive command → process → send response (non-blocking select)

Test results

Added test_ableton_mcp.py — 39 integration tests covering all commands:

Results: 39/39 passed, 0 failed, 1 skipped
All tests passed.

Tests cover: get_session_info, get_track_info, create_midi_track, set_track_name, create_clip, add_notes_to_clip, set_clip_name, set_tempo, browser commands, error handling, and sequential stress test.

Compatibility

  • Tested on Ableton Live 10.1.43 / macOS
  • MCP client: Claude Code with uvx ableton-mcp
  • Not tested on Live 11/12 (should work, but the threadless approach may behave differently)

Notes

  • Only one MCP client at a time (same as original)
  • Session cache resets after any write command and rebuilds on next tick

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor

    • Switched to a single-threaded, non-blocking networking loop for client communication and command handling.
  • Bug Fixes / Changes

    • Streamlined browser API to only provide tree retrieval and path-based item queries; reduced browser response verbosity.
    • Session/track/clip operations now run inline with display updates and maintain a cached session snapshot.
  • Tests

    • Added an integration test script exercising session, track, clip, tempo and browser flows.
  • Documentation

    • Added usage and implementation guidance for the local TCP MCP protocol and testing.

Yago Bolivar and others added 2 commits May 31, 2026 21:08
…lity

Live 10 uses an embedded Python 2.7 runtime where background threads suffer
from severe GIL starvation — threads can wait 60-90 seconds before getting
CPU time, causing the MCP server's 15-second timeout to fire before any
response is sent.

This rewrite moves all socket I/O and Live API calls into update_display(),
a hook guaranteed to be called by Live every ~100ms on its main thread.
No background threads are used at all.

Changes:
- Replace server/client threads with non-blocking select() in update_display()
- Cache session info on first update_display() tick (safe: main thread)
- Call Live API directly in _process_command() (no schedule_message/queues)
- Add Python 2/3 compatibility (bytes/str encoding, Queue import)
- Remove threading, queue, time imports (no longer needed)
- Script is ~360 lines vs original ~1000+ lines

Tested on Ableton Live 10.1.43 / macOS with Claude Code MCP client.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 31, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Refactors AbletonMCP to a single-threaded, non-blocking select loop inside update_display(), simplifies command routing and browser responses, and adds an integration test script plus CLAUDE.md guidance.

Changes

Single-Threaded Server Refactor with Integration Test Suite

Layer / File(s) Summary
Network Architecture & Main Loop
AbletonMCP_Remote_Script/__init__.py
Moves from threading/queue imports to select; __init__ initializes socket state and calls _start_server() directly; disconnect() closes sockets without thread management; update_display() becomes the central non-blocking loop accepting connections, reading UTF-8 JSON, processing commands, and sending responses.
Command Processing & Cache Management
AbletonMCP_Remote_Script/__init__.py
_process_command() routes a reduced command set with centralized exception handling; state-mutating commands clear _session_cache to force rebuilds, eliminating the prior worker-thread-to-main-thread task-queue pattern.
Session, Track, and Clip Operations
AbletonMCP_Remote_Script/__init__.py
_get_session_info() reads from cache or raises; _get_track_info() enumerates clip slots and devices; track/clip creation, MIDI insertion, naming, and tempo mutations preserve core behavior but execute synchronously from the main loop.
Browser API Simplification
AbletonMCP_Remote_Script/__init__.py
get_browser_tree() and get_browser_items_at_path() return simplified {type, categories} and {path, name, items} shapes; helper functions _find_browser_item_by_uri() and _get_device_type() remain for URI lookup and device classification.
Test Infrastructure & Helpers
test_ableton_mcp.py
Introduces test configuration, result tracking, send_command() helper with timeout/retry, and reporting functions; connect() establishes socket connection with error handling.
Core Functionality Tests
test_ableton_mcp.py
12-step test suite covering session info, track info with boundary cases, error handling, MIDI track creation, track naming, clip creation/naming, MIDI note insertion, tempo setting, and browser navigation; validates all core server operations through JSON API.
Browser and Performance Tests
test_ableton_mcp.py
Tests get_browser_tree categories, get_browser_items_at_path for valid and invalid paths, and stress-tests rapid session-info calls to verify single-threaded throughput.
Test Results & Entrypoint
test_ableton_mcp.py
Aggregates test results, prints colored pass/fail/skip summary, and exits with status code 0 on success or 1 on failure.
Developer guidance
CLAUDE.md
Documents architecture, JSON chunk accumulation protocol, run/install/test commands, Remote Script installation, and Live 10-specific non-blocking/encoding constraints.

Sequence Diagram

sequenceDiagram
  participant MCPClient as MCP Client
  participant Remote as AbletonMCP (update_display)
  participant Song as Ableton Song API

  MCPClient->>Remote: TCP connect + send JSON request
  Remote->>Remote: accumulate buffer, parse JSON
  Remote->>Remote: _process_command()
  Remote->>Song: invoke song/track/clip/browser operations (mutate/read)
  Song-->>Remote: return results/state
  Remote-->>MCPClient: send JSON response
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • ahujasid/ableton-mcp#4: Modifies TCP/network handling and JSON parsing mechanics in the same AbletonMCP networking area.

Poem

🐰 I hopped in to trim the threads,
Rewired sockets where data treads.
One loop now listens, parses, replies,
Tests march on with steady tries.
A tiny rabbit cheers the run—bravo, tidy webs!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and specifically describes the main change: rewriting the architecture to remove threading for Ableton Live 10 Python 2.7 compatibility, centered on the threadless update_display approach.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Rewrite for Ableton Live 10 Python 2.7 compatibility — threadless architecture

🐞 Bug fix ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Eliminate GIL starvation by removing all background threads
• Move socket I/O and Live API calls to update_display() main thread hook
• Replace queue-based scheduling with direct API calls (safe on main thread)
• Reduce codebase from ~1000 to ~360 lines with simplified architecture
• Add comprehensive 39-test integration suite validating all commands
Diagram
flowchart LR
  A["update_display<br/>main thread ~100ms"] --> B["Build session cache<br/>first tick only"]
  A --> C["Non-blocking select<br/>timeout=0"]
  C --> D["Accept client<br/>if available"]
  C --> E["Read from client<br/>parse JSON"]
  E --> F["_process_command<br/>direct Live API calls"]
  F --> G["Send response<br/>non-blocking"]
  G --> H["Reset cache<br/>on state change"]

Loading

Grey Divider

File Changes

1. AbletonMCP_Remote_Script/__init__.py Bug fix, enhancement +271/-974

Threadless socket I/O architecture with main-thread session cache

• Remove all threading, queue, and time imports; add select for non-blocking I/O
• Replace server/client thread architecture with single-connection model in update_display()
• Implement session cache built on first update_display() tick and invalidated after state changes
• Simplify all command handlers by removing schedule_message() and queue gymnastics
• Add Python 2/3 compatibility for bytes/string encoding with try/except blocks
• Reduce code from ~1062 to ~359 lines while maintaining all functionality
• Streamline browser tree and path navigation methods with simplified error handling

AbletonMCP_Remote_Script/init.py


2. test_ableton_mcp.py 🧪 Tests +281/-0

Comprehensive integration test suite for all MCP operations

• Add 39 integration tests covering all MCP commands and error cases
• Test session info retrieval, track creation, clip management, tempo control
• Validate browser tree and path navigation functionality
• Include sequential stress test (5 rapid commands) and error handling verification
• Provide colored output (PASS/FAIL/SKIP) and detailed timing metrics
• Verify response times under 1s and track count increments after creation

test_ableton_mcp.py


Grey Divider

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented May 31, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (4) 📘 Rule violations (0)

Grey Divider


Action required

1. Nonblocking sendall disconnects 🐞 Bug ☼ Reliability
Description
update_display() sets the client socket to non-blocking but still uses sendall() without handling
EWOULDBLOCK/partial writes, so responses can raise and the code will close the client connection.
This will show up as intermittent disconnects especially with larger JSON responses.
Code

AbletonMCP_Remote_Script/init.py[R82-120]

Evidence
The code explicitly makes the client non-blocking and then immediately uses sendall() without any
writable-select or partial-send buffering; any send failure falls into the broad exception handler
which closes the socket.

AbletonMCP_Remote_Script/init.py[78-120]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`update_display()` sets the accepted client socket to non-blocking, but then calls `sendall()` directly. On non-blocking sockets, `sendall()` can raise when the kernel send buffer is full and/or may require partial-write retry logic.

### Issue Context
This runs on Ableton’s main thread; we need to avoid blocking, but also avoid disconnecting clients on transient backpressure. The current `except Exception` path closes the socket immediately on any send error.

### Fix Focus Areas
- Implement an outgoing buffer (`self._out_buffer`) and send only when the socket is writable.
- Use `select.select([], [self._client], [], 0)` to check writability before sending.
- On `EWOULDBLOCK`/`EAGAIN`, keep remaining bytes in the buffer and retry next tick.
- Only close the client on real fatal socket errors.

#### Code locations
- AbletonMCP_Remote_Script/__init__.py[78-120]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. JSON framing breaks pipelining 🐞 Bug ≡ Correctness
Description
The server assumes the entire receive buffer is exactly one JSON object; if multiple commands arrive
in a single recv (or a full JSON plus extra bytes), json.loads() fails and the buffer is never
recovered. This can permanently block command processing for the connected client.
Code

AbletonMCP_Remote_Script/init.py[R102-116]

Evidence
The parsing logic attempts exactly one json.loads on the full buffer and on any ValueError does
nothing other than keep accumulating bytes, which cannot handle extra trailing data or multiple JSON
objects in the stream.

AbletonMCP_Remote_Script/init.py[102-116]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`json.loads(self._buffer)` is used as the only parse strategy, and any `ValueError` is treated as “incomplete JSON”. This fails when there are extra bytes after a complete JSON value (e.g., concatenated `{...}{...}`), and also provides no recovery path for invalid JSON.

### Issue Context
TCP is a stream: multiple writes can coalesce into one read. The server should support either:
- newline-delimited JSON (`\n` as delimiter), or
- length-prefixed frames, or
- incremental parsing using `json.JSONDecoder().raw_decode` while preserving any remaining bytes.

### Fix Focus Areas
- Replace `json.loads(self._buffer)` with incremental decode (raw_decode) and handle leftover bytes.
- Loop to process multiple complete commands in one tick (bounded to avoid starving Ableton UI).
- Add a max buffer size / reset-on-invalid strategy to avoid unbounded growth.

#### Code locations
- AbletonMCP_Remote_Script/__init__.py[102-116]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Clip index validation regression 🐞 Bug ≡ Correctness
Description
_set_clip_name(), _fire_clip(), and _stop_clip() no longer perform explicit track/clip index bounds
checks, unlike other handlers (e.g., _set_track_name/_create_clip). This creates inconsistent API
behavior and allows negative indices to be treated as valid Python list indexing.
Code

AbletonMCP_Remote_Script/init.py[R232-253]

Evidence
The three clip-related handlers index into tracks/clip_slots without any bounds checks, while other
nearby handlers still enforce bounds explicitly, demonstrating an inconsistency introduced by the
refactor.

AbletonMCP_Remote_Script/init.py[232-253]
AbletonMCP_Remote_Script/init.py[199-215]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Some command handlers directly index `self._song.tracks[...]` / `track.clip_slots[...]` without validating indices, while others explicitly reject `< 0` and `>= len(...)`. This inconsistency can lead to surprising behavior (e.g., `-1` selects the last track) and less clear error messages.

### Issue Context
The existing API style in this file rejects negative indices and provides clear `IndexError("... out of range")` for invalid indices.

### Fix Focus Areas
- Add the same `if track_index < 0 or track_index >= len(self._song.tracks): raise IndexError(...)` checks used elsewhere.
- Add `clip_index` bounds checks against `len(track.clip_slots)`.
- Apply to `_set_clip_name`, `_fire_clip`, and `_stop_clip`.

#### Code locations
- AbletonMCP_Remote_Script/__init__.py[232-253]
- AbletonMCP_Remote_Script/__init__.py[199-215] (reference pattern)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

4. Browser lookup errors hidden 🐞 Bug ◔ Observability
Description
_find_browser_item_by_uri() now catches all exceptions and returns None without logging, making it
impossible to distinguish “not found” from an internal traversal error. This reduces observability
when load_browser_item fails.
Code

AbletonMCP_Remote_Script/init.py[R275-294]

Evidence
The helper’s except: block drops all exception details and returns None, so traversal failures
become silent and indistinguishable from a legitimate “not found”.

AbletonMCP_Remote_Script/init.py[275-294]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
A bare `except:` in `_find_browser_item_by_uri` swallows all errors and returns `None`, hiding root causes and making debugging browser traversal failures much harder.

### Issue Context
Callers already treat `None` as “not found”; we still want to log unexpected exceptions (at least `Exception as e` + traceback) so failures are diagnosable.

### Fix Focus Areas
- Replace `except:` with `except Exception as e:`.
- Log `traceback.format_exc()` (or at minimum `str(e)`) before returning `None`.

#### Code locations
- AbletonMCP_Remote_Script/__init__.py[275-294]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Qodo Logo

Comment on lines +82 to +120
self._client, addr = self._server.accept()
self._client.setblocking(False)
self._buffer = ""
self.log_message("Client connected from " + str(addr))
self.show_message("AbletonMCP: client connected")
except Exception as e:
self.log_message("Accept error: " + str(e))
return

# Read from existing client
try:
r, _, _ = select.select([self._client], [], [], 0)
if not r:
return
data = self._client.recv(8192)
if not data:
self.log_message("Client disconnected")
self._client.close()
self._client = None
return
try:
self._buffer += data.decode("utf-8")
except AttributeError:
self._buffer += data

try:
command = json.loads(self._buffer)
self._buffer = ""
self.log_message("Received command: " + str(command.get("type", "?")))
response = self._process_command(command)
resp_bytes = json.dumps(response).encode("utf-8")
self._client.sendall(resp_bytes)
except ValueError:
pass # incomplete JSON, wait for more
except Exception as e:
self.log_message("Client error: " + str(e))
try: self._client.close()
except: pass
self._client = None

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Nonblocking sendall disconnects 🐞 Bug ☼ Reliability

update_display() sets the client socket to non-blocking but still uses sendall() without handling
EWOULDBLOCK/partial writes, so responses can raise and the code will close the client connection.
This will show up as intermittent disconnects especially with larger JSON responses.
Agent Prompt
### Issue description
`update_display()` sets the accepted client socket to non-blocking, but then calls `sendall()` directly. On non-blocking sockets, `sendall()` can raise when the kernel send buffer is full and/or may require partial-write retry logic.

### Issue Context
This runs on Ableton’s main thread; we need to avoid blocking, but also avoid disconnecting clients on transient backpressure. The current `except Exception` path closes the socket immediately on any send error.

### Fix Focus Areas
- Implement an outgoing buffer (`self._out_buffer`) and send only when the socket is writable.
- Use `select.select([], [self._client], [], 0)` to check writability before sending.
- On `EWOULDBLOCK`/`EAGAIN`, keep remaining bytes in the buffer and retry next tick.
- Only close the client on real fatal socket errors.

#### Code locations
- AbletonMCP_Remote_Script/__init__.py[78-120]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +102 to +116
try:
self._buffer += data.decode("utf-8")
except AttributeError:
self._buffer += data

try:
command = json.loads(self._buffer)
self._buffer = ""
self.log_message("Received command: " + str(command.get("type", "?")))
response = self._process_command(command)
resp_bytes = json.dumps(response).encode("utf-8")
self._client.sendall(resp_bytes)
except ValueError:
pass # incomplete JSON, wait for more
except Exception as e:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. Json framing breaks pipelining 🐞 Bug ≡ Correctness

The server assumes the entire receive buffer is exactly one JSON object; if multiple commands arrive
in a single recv (or a full JSON plus extra bytes), json.loads() fails and the buffer is never
recovered. This can permanently block command processing for the connected client.
Agent Prompt
### Issue description
`json.loads(self._buffer)` is used as the only parse strategy, and any `ValueError` is treated as “incomplete JSON”. This fails when there are extra bytes after a complete JSON value (e.g., concatenated `{...}{...}`), and also provides no recovery path for invalid JSON.

### Issue Context
TCP is a stream: multiple writes can coalesce into one read. The server should support either:
- newline-delimited JSON (`\n` as delimiter), or
- length-prefixed frames, or
- incremental parsing using `json.JSONDecoder().raw_decode` while preserving any remaining bytes.

### Fix Focus Areas
- Replace `json.loads(self._buffer)` with incremental decode (raw_decode) and handle leftover bytes.
- Loop to process multiple complete commands in one tick (bounded to avoid starving Ableton UI).
- Add a max buffer size / reset-on-invalid strategy to avoid unbounded growth.

#### Code locations
- AbletonMCP_Remote_Script/__init__.py[102-116]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
test_ableton_mcp.py (1)

129-129: ⚡ Quick win

Extract the repeated result-unwrapping into a helper.

The pattern json.loads(r["result"]) if isinstance(r.get("result"), str) else r.get("result", {}) is duplicated across ~15 call sites (Lines 129, 142, 164, 171, 180, 190, 207, 218, 226, 232, 239, 244, 248). A single helper removes the duplication and makes the assertions easier to read and maintain.

♻️ Proposed helper + representative replacement

Add near the top (e.g., after results):

def result_of(response):
    res = response.get("result")
    return json.loads(res) if isinstance(res, str) else (res or {})

Then replace each occurrence, e.g.:

-    info = json.loads(r["result"]) if isinstance(r.get("result"), str) else r.get("result", {})
+    info = result_of(r)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test_ableton_mcp.py` at line 129, Extract the repeated unwrapping of response
results into a small helper named result_of(response) that returns
json.loads(res) when response.get("result") is a str, otherwise returns the
result or {}; add this helper near the top (after results) and replace each
occurrence of the expression json.loads(r["result"]) if
isinstance(r.get("result"), str) else r.get("result", {}) (and similar uses of
r.get("result")) with result_of(r) across the ~15 call sites so assertions
become result_of(r) instead of the inline conditional.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@AbletonMCP_Remote_Script/__init__.py`:
- Around line 232-238: Add explicit bounds checks in the clip handler methods
(_set_clip_name, _fire_clip, and _stop_clip) so negative indices are not
accepted and out-of-range indices raise IndexError: verify 0 <= track_index <
len(self._song.tracks) and then get track = self._song.tracks[track_index];
verify 0 <= clip_index < len(track.clip_slots) before accessing
track.clip_slots[clip_index]; if either check fails raise IndexError with a
clear message. Ensure you keep the existing slot.has_clip check after these
bounds validations.
- Around line 107-115: Your recv loop currently does json.loads(self._buffer)
and swallows ValueError, which causes stuck buffers when multiple JSON objects
or malformed data are present; replace that single-shot json.loads call with
incremental decoding using json.JSONDecoder().raw_decode to parse and consume
exactly one JSON object from self._buffer, call _process_command on the decoded
object, send the response, then remove the consumed slice from self._buffer and
continue parsing any remaining data; treat a ValueError from raw_decode as
“incomplete data” (break to await more recv) but treat other parse errors as
malformed input — log the error, drop/clear the offending bytes from
self._buffer (or close the connection) to avoid infinite retry loops;
alternatively, implement explicit framing (delimiter or length prefix) around
sends/recv instead of relying on stream boundaries.

---

Nitpick comments:
In `@test_ableton_mcp.py`:
- Line 129: Extract the repeated unwrapping of response results into a small
helper named result_of(response) that returns json.loads(res) when
response.get("result") is a str, otherwise returns the result or {}; add this
helper near the top (after results) and replace each occurrence of the
expression json.loads(r["result"]) if isinstance(r.get("result"), str) else
r.get("result", {}) (and similar uses of r.get("result")) with result_of(r)
across the ~15 call sites so assertions become result_of(r) instead of the
inline conditional.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6f55f50d-21a0-492d-a8ed-7a363cb95812

📥 Commits

Reviewing files that changed from the base of the PR and between e008328 and 32b984b.

📒 Files selected for processing (2)
  • AbletonMCP_Remote_Script/__init__.py
  • test_ableton_mcp.py

Comment on lines +107 to +115
try:
command = json.loads(self._buffer)
self._buffer = ""
self.log_message("Received command: " + str(command.get("type", "?")))
response = self._process_command(command)
resp_bytes = json.dumps(response).encode("utf-8")
self._client.sendall(resp_bytes)
except ValueError:
pass # incomplete JSON, wait for more

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="AbletonMCP_Remote_Script/__init__.py"

echo "== File header =="
head -n 40 "$FILE"

echo "== Lines 80-160 =="
nl -ba "$FILE" | sed -n '80,160p'

echo "== Find index guards =="
rg -n "def _set_track_name|def _set_clip_name|def _create_clip|def _add_notes_to_clip|def _fire_clip|def _stop_clip" "$FILE" -n
rg -n "reject negative|negative|< 0|index < 0" "$FILE"

echo "== Browser APIs =="
rg -n "def get_browser_tree|def get_browser_items_at_path|Audio Effects|MIDI Effects|audio_effects|midi_effects" "$FILE"
nl -ba "$FILE" | sed -n '160,280p' || true

Repository: ahujasid/ableton-mcp

Length of output: 1442


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="AbletonMCP_Remote_Script/__init__.py"

echo "== Confirm file exists =="
ls -la "$FILE"

echo "== Show full snippet around lines 90-140 (no nl dependency) =="
awk 'NR>=90 && NR<=140 {printf "%5d:%s\n", NR, $0}' "$FILE"

echo "== Find recv loop / JSON parsing =="
rg -n "recv|_buffer|json\.loads|sendall|raw_decode" "$FILE"

echo "== Show any negative index guards =="
rg -n "index < 0|< 0|negative|reject" "$FILE"

echo "== Browser tree/path contract =="
rg -n "get_browser_tree|get_browser_items_at_path|Audio Effects|MIDI Effects|audio_effects|midi_effects" "$FILE"

Repository: ahujasid/ableton-mcp

Length of output: 5009


🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE="AbletonMCP_Remote_Script/__init__.py"

echo "== Snippet around negative index guards =="
awk 'NR>=150 && NR<=290 {printf "%5d:%s\n", NR, $0}' "$FILE"

echo "== Snippet around browser functions =="
awk 'NR>=290 && NR<=380 {printf "%5d:%s\n", NR, $0}' "$FILE"

echo "== List function definitions for mapping guards =="
rg -n "def _set_track_name|def _create_clip|def _add_notes_to_clip|def _set_clip_name|def _fire_clip|def _stop_clip|def get_browser_tree|def get_browser_items_at_path" "$FILE"

Repository: ahujasid/ableton-mcp

Length of output: 12196


Fix TCP JSON framing: don’t treat every ValueError as “incomplete JSON”.
In AbletonMCP_Remote_Script/__init__.py recv loop, _buffer is appended with each recv() and json.loads(self._buffer) is attempted; on except ValueError the code passes without consuming/clearing _buffer. Since ValueError can also represent “extra data” (multiple JSON objects coalesced) or other malformed payloads, the same bad combined buffer will be retried repeatedly and the server can stop responding to further commands. Implement proper framing (delimiter/length prefix) or incremental decoding that consumes exactly one JSON object at a time and rejects/clears invalid payloads.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@AbletonMCP_Remote_Script/__init__.py` around lines 107 - 115, Your recv loop
currently does json.loads(self._buffer) and swallows ValueError, which causes
stuck buffers when multiple JSON objects or malformed data are present; replace
that single-shot json.loads call with incremental decoding using
json.JSONDecoder().raw_decode to parse and consume exactly one JSON object from
self._buffer, call _process_command on the decoded object, send the response,
then remove the consumed slice from self._buffer and continue parsing any
remaining data; treat a ValueError from raw_decode as “incomplete data” (break
to await more recv) but treat other parse errors as malformed input — log the
error, drop/clear the offending bytes from self._buffer (or close the
connection) to avoid infinite retry loops; alternatively, implement explicit
framing (delimiter or length prefix) around sends/recv instead of relying on
stream boundaries.

Comment on lines 232 to +238
def _set_clip_name(self, track_index, clip_index, name):
"""Set the name of a clip"""
try:
if track_index < 0 or track_index >= len(self._song.tracks):
raise IndexError("Track index out of range")

track = self._song.tracks[track_index]

if clip_index < 0 or clip_index >= len(track.clip_slots):
raise IndexError("Clip index out of range")

clip_slot = track.clip_slots[clip_index]

if not clip_slot.has_clip:
raise Exception("No clip in slot")

clip = clip_slot.clip
clip.name = name

result = {
"name": clip.name
}
return result
except Exception as e:
self.log_message("Error setting clip name: " + str(e))
raise

track = self._song.tracks[track_index]
slot = track.clip_slots[clip_index]
if not slot.has_clip:
raise Exception("No clip in slot")
slot.clip.name = name
return {"name": slot.clip.name}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the file and relevant methods
ls -R
sed -n '180,320p' AbletonMCP_Remote_Script/__init__.py | cat
rg -n "def _set_clip_name|def _fire_clip|def _stop_clip|_set_track_name|_create_clip|_add_notes_to_clip" AbletonMCP_Remote_Script/__init__.py

Repository: ahujasid/ableton-mcp

Length of output: 7909


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '200,290p' AbletonMCP_Remote_Script/__init__.py | nl -ba | sed -n '1,120p'
rg -n "def _set_clip_name|def _fire_clip|def _stop_clip|def _set_track_name|def _create_clip|def _add_notes_to_clip" AbletonMCP_Remote_Script/__init__.py

Repository: ahujasid/ableton-mcp

Length of output: 107


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the exact regions around the referenced line numbers
nl -ba AbletonMCP_Remote_Script/__init__.py | sed -n '210,270p'

# Also show a bit more around fire/stop if they are near
nl -ba AbletonMCP_Remote_Script/__init__.py | sed -n '270,340p'

# Find all relevant defs
rg -n "def _set_clip_name|def _fire_clip|def _stop_clip|def _set_track_name|def _create_clip|def _add_notes_to_clip" AbletonMCP_Remote_Script/__init__.py

Repository: ahujasid/ableton-mcp

Length of output: 107


🏁 Script executed:

#!/bin/bash
set -euo pipefail

nl -ba AbletonMCP_Remote_Script/__init__.py | sed -n '210,270p'
nl -ba AbletonMCP_Remote_Script/__init__.py | sed -n '270,340p'

rg -n "def _set_clip_name|def _fire_clip|def _stop_clip|def _set_track_name|def _create_clip|def _add_notes_to_clip" AbletonMCP_Remote_Script/__init__.py

Repository: ahujasid/ableton-mcp

Length of output: 107


Reject negative/out-of-range track_index and clip_index in clip handlers

_set_clip_name, _fire_clip, and _stop_clip index self._song.tracks[track_index] / track.clip_slots[clip_index] directly; in Python, -1 will target the last track/clip instead of erroring. Add bounds checks and raise IndexError for both indexes.

Suggested guard pattern
 def _set_clip_name(self, track_index, clip_index, name):
+        if track_index < 0 or track_index >= len(self._song.tracks):
+            raise IndexError("Track index out of range")
         track = self._song.tracks[track_index]
+        if clip_index < 0 or clip_index >= len(track.clip_slots):
+            raise IndexError("Clip index out of range")
         slot = track.clip_slots[clip_index]
         if not slot.has_clip:
             raise Exception("No clip in slot")
         slot.clip.name = name
         return {"name": slot.clip.name}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@AbletonMCP_Remote_Script/__init__.py` around lines 232 - 238, Add explicit
bounds checks in the clip handler methods (_set_clip_name, _fire_clip, and
_stop_clip) so negative indices are not accepted and out-of-range indices raise
IndexError: verify 0 <= track_index < len(self._song.tracks) and then get track
= self._song.tracks[track_index]; verify 0 <= clip_index < len(track.clip_slots)
before accessing track.clip_slots[clip_index]; if either check fails raise
IndexError with a clear message. Ensure you keep the existing slot.has_clip
check after these bounds validations.

Comment on lines +314 to +318
for attr, label in [("instruments", "Instruments"), ("sounds", "Sounds"), ("drums", "Drums"), ("audio_effects", "Audio Effects"), ("midi_effects", "MIDI Effects")]:
if (category_type == "all" or category_type == attr) and hasattr(browser, attr):
try:
item = getattr(app.browser, attr)
if hasattr(item, 'children') or hasattr(item, 'name'):
category = process_item(item)
if category:
category["name"] = attr.capitalize()
result["categories"].append(category)
except Exception as e:
self.log_message("Error processing {0}: {1}".format(attr, str(e)))

self.log_message("Browser tree generated for {0} with {1} root categories".format(
category_type, len(result['categories'])))
item = getattr(browser, attr)
result["categories"].append({"name": label, "uri": getattr(item, "uri", None), "is_folder": True})

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Return a canonical category token from get_browser_tree().

get_browser_tree() returns display names like Audio Effects and MIDI Effects, but get_browser_items_at_path() only accepts audio_effects and midi_effects. A client walking the returned tree cannot browse those categories without hard-coded special cases. Expose the path token in the tree response, or normalize spaces on lookup.

Also applies to: 333-339

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
CLAUDE.md (2)

47-52: ⚡ Quick win

Consider adding Windows and Linux paths.

The installation instructions only provide the macOS path. Adding paths for other platforms would improve accessibility:

  • Windows: %USERPROFILE%\Documents\Ableton\User Library\Remote Scripts\AbletonMCP\
  • Linux: ~/Documents/Ableton/User Library/Remote Scripts/AbletonMCP/ (or Wine equivalent)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CLAUDE.md` around lines 47 - 52, The "Remote Script installation" section
only lists the macOS path; add equivalent Windows and Linux paths and a short
Wine note so users on other platforms can install the Remote Script. Update the
paragraph that currently shows `~/Music/Ableton/User Library/Remote
Scripts/AbletonMCP/` to include the Windows path
`%USERPROFILE%\Documents\Ableton\User Library\Remote Scripts\AbletonMCP\` and
the Linux path `~/Documents/Ableton/User Library/Remote Scripts/AbletonMCP/`
(and mention "or Wine equivalent" for Linux users), and ensure the instruction
to set a Control Surface slot to `AbletonMCP` remains unchanged.

9-15: 💤 Low value

Add language specifier to fenced code block.

Markdown linters prefer an explicit language tag. For ASCII diagrams, use ```text instead of ```.

📝 Proposed fix
-```
+```text
 Claude Code / MCP client
         ↓  MCP protocol
   MCP_Server/server.py       ← Python 3, FastMCP, runs via `uvx ableton-mcp`
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CLAUDE.md` around lines 9 - 15, Update the fenced code block in CLAUDE.md to
include an explicit language specifier (use `text`) so the ASCII diagram is
properly marked; locate the triple-backtick block containing the MCP diagram and
change the opening fence from ``` to ```text to satisfy markdown linters.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@CLAUDE.md`:
- Around line 47-52: The "Remote Script installation" section only lists the
macOS path; add equivalent Windows and Linux paths and a short Wine note so
users on other platforms can install the Remote Script. Update the paragraph
that currently shows `~/Music/Ableton/User Library/Remote Scripts/AbletonMCP/`
to include the Windows path `%USERPROFILE%\Documents\Ableton\User Library\Remote
Scripts\AbletonMCP\` and the Linux path `~/Documents/Ableton/User Library/Remote
Scripts/AbletonMCP/` (and mention "or Wine equivalent" for Linux users), and
ensure the instruction to set a Control Surface slot to `AbletonMCP` remains
unchanged.
- Around line 9-15: Update the fenced code block in CLAUDE.md to include an
explicit language specifier (use `text`) so the ASCII diagram is properly
marked; locate the triple-backtick block containing the MCP diagram and change
the opening fence from ``` to ```text to satisfy markdown linters.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4a13f722-31be-4837-aa24-abda0dbe21b2

📥 Commits

Reviewing files that changed from the base of the PR and between 32b984b and 9908e6f.

📒 Files selected for processing (1)
  • CLAUDE.md

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.

1 participant