Conversation
…architecture Speed improvements: - Async TCP connection (asyncio) replacing blocking sockets - Remove all time.sleep() calls from command pipeline - Response caching with TTL (2s session, 60s browser) - get_full_session_state: complete session snapshot in 1 call vs N+1 - Batch command support (send array of commands in single TCP transmission) New tools (15 → 67): - Track management: create/delete/duplicate tracks, volume, pan, mute, solo, arm, sends - Clip operations: get/remove notes, delete, duplicate, loop settings, quantize - Scene management: create/delete/duplicate/fire scenes, stop all clips - Device control: get/set parameters by index or name, toggle, delete devices - Transport extensions: undo/redo, metronome, loop, capture MIDI, tap tempo, record - Browser improvements: text search, aggressive caching - AI music theory: 23 chord types, 15 scales, 10 rhythm styles, chord progressions, basslines, melodies, harmony analysis Architecture: - Modular tool system: 8 independent modules under MCP_Server/tools/ - Async connection layer with auto-reconnection (MCP_Server/connection.py) - TTL-based response cache (MCP_Server/cache.py) - Command registry pattern in Remote Script (replaces if/elif chain) - Remote Script: 61 command handlers with batch support Also adds: - install.sh for automated installation - CLAUDE.md project guide - Updated README with full documentation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…vitation to contribute Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…uddle Major overhaul: 67 tools, async, AI music theory
Based on investigation of: - Ableton Live Python API (LOM) capabilities and limitations - Context-aware AI music generation feasibility - Mock testing approaches - Developer experience improvements Key findings: - Automation envelopes: viable for session clips only (arrangement returns None) - Audio clip warping/pitch/gain: fully viable - Arrangement view: read + create viable, but clip position is read-only - Export/render: not possible via API - All AI features (key detection, variations, templates, mixing): pure Python, no ML needed - Testing: architecture already supports dependency injection for mocking Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe project undergoes a major architectural refactoring: replacing a monolithic server with modular, async-based infrastructure. New async TCP connection abstraction, TTL-based response caching, and eight specialized tool modules are introduced, delegating MCP endpoint registrations from server.py to organized tool submodules while updating documentation and installation mechanisms. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Review Summary by QodoMajor architectural refactor to modular async design with 67+ tools and comprehensive documentation
WalkthroughsDescription• **Major architectural refactor**: Transformed monolithic server.py (660 lines) into modular architecture with 8 specialized tool modules (session_tools, track_tools, clip_tools, scene_tools, device_tools, transport_tools, browser_tools, ai_tools) • **Async connection layer**: Replaced blocking socket implementation with non-blocking asyncio-based AbletonConnection class supporting automatic reconnection and batch commands • **Response caching system**: Implemented TTL-based ResponseCache for efficient repeated queries, reducing redundant socket round-trips • **Comprehensive tool coverage**: Added 67+ tools across 8 categories including track management, clip operations, scene control, device parameters, transport/playback, browser navigation, session queries, and AI-powered music theory generation • **AI music theory module**: New 1128-line ai_tools.py providing 9 tools for note conversion, chord/scale generation, progressions, rhythm patterns, basslines, melodies, and chord suggestions • **Remote Script enhancements**: Refactored command registry with 60+ handler implementations, batch command support, and improved error handling in AbletonMCP_Remote_Script/__init__.py • **Installation automation**: Added install.sh script for automated setup of both MCP Server and Ableton Remote Script with multi-version detection • **Documentation overhaul**: Completely restructured README for user-centric focus, added comprehensive ROADMAP.md (v2.0-v2.3 features), and created CLAUDE.md developer guide • **Version bump**: Updated to v1.0.0 reflecting production-ready status Diagramflowchart LR
A["Monolithic<br/>server.py<br/>660 lines"] -->|"Refactor to<br/>modular architecture"| B["MCP Server<br/>77 lines"]
B -->|"Registers"| C["8 Tool Modules"]
C -->|"session_tools"| D["Session Queries"]
C -->|"track_tools"| E["Track Management"]
C -->|"clip_tools"| F["Clip Operations"]
C -->|"ai_tools"| G["Music Theory<br/>Generation"]
B -->|"Uses"| H["Async Connection<br/>asyncio streams"]
B -->|"Caches with"| I["ResponseCache<br/>TTL-based"]
H -->|"Communicates with"| J["Ableton Remote<br/>Script"]
J -->|"Handles 60+<br/>commands"| K["Ableton Live"]
File Changes1. AbletonMCP_Remote_Script/__init__.py
|
Code Review by Qodo
1. Pitch span off-by-one
|
| from_time: float = 0.0, | ||
| time_span: float = 999.0, | ||
| from_pitch: int = 0, | ||
| pitch_span: int = 127, |
There was a problem hiding this comment.
1. Pitch span off-by-one 🐞 Bug ✓ Correctness
remove_notes_from_clip defaults pitch_span=127, but Ableton’s API treats pitch_span as a count, so the default call fails to remove MIDI pitch 127 notes. This causes silent partial note deletion when callers rely on defaults.
Agent Prompt
## Issue description
`MCP_Server/tools/clip_tools.py::remove_notes_from_clip` defaults `pitch_span` to 127, but the Remote Script/Ableton API uses `pitch_span` as a count, so the default call won’t cover pitch 127.
## Issue Context
Remote script handler defaults `pitch_span` to 128 and forwards it directly to `clip.remove_notes(from_time, from_pitch, time_span, pitch_span)`.
## Fix Focus Areas
- MCP_Server/tools/clip_tools.py[177-205]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| async def set_loop(enabled: bool, start: float = 0.0, length: float = 4.0) -> str: | ||
| """Set loop on/off and configure the loop range in beats. | ||
|
|
||
| Args: | ||
| enabled: True to enable looping, False to disable it. | ||
| start: Loop start position in beats. Defaults to 0.0. | ||
| length: Loop length in beats. Defaults to 4.0. | ||
| """ | ||
| try: | ||
| conn = await get_connection() | ||
| result = await conn.send_command("set_loop", {"enabled": enabled, "start": start, "length": length}) | ||
| cache.invalidate_all() |
There was a problem hiding this comment.
2. Loop toggle resets range 🐞 Bug ✓ Correctness
set_loop(enabled=False) always sends start=0.0 and length=4.0, so disabling looping unintentionally overwrites the current loop range. This destroys the user’s existing loop_start/loop_length even when they only intended to toggle looping off.
Agent Prompt
## Issue description
`set_loop` always sends `start` and `length` due to default values, which causes unintended mutation of Ableton’s loop range when the user only wants to enable/disable looping.
## Issue Context
Remote script only mutates `loop_start`/`loop_length` when received values are not None.
## Fix Focus Areas
- MCP_Server/tools/transport_tools.py[126-139]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| items = kit_items if isinstance(kit_items, list) else kit_items.get("items", []) | ||
| for item in items: | ||
| if item.get("is_loadable") or item.get("uri"): | ||
| loadable_uri = item.get("uri") | ||
| break |
There was a problem hiding this comment.
3. Loads non-loadable kit 🐞 Bug ✓ Correctness
load_drum_kit selects the first item that has a uri even when is_loadable is false, so it can try to load folders/non-loadable items. This can load the wrong content or fail unpredictably depending on browser structure ordering.
Agent Prompt
## Issue description
`load_drum_kit` currently picks the first item with a URI even if it’s not loadable, which can attempt to load folders or other non-loadable browser nodes.
## Issue Context
`get_browser_items_at_path` returns both `is_loadable` and `uri`; `uri` alone is not a safe proxy for loadability.
## Fix Focus Areas
- MCP_Server/tools/browser_tools.py[177-183]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| client.settimeout(None) | ||
| buffer = '' | ||
|
|
||
| try: | ||
| while self.running: | ||
| try: | ||
| # Receive data | ||
| data = client.recv(8192) | ||
|
|
There was a problem hiding this comment.
4. Client threads hang shutdown 🐞 Bug ⛯ Reliability
Remote Script client threads can block forever in recv() because the socket is put into fully blocking mode, so disconnect() setting self.running=False may not terminate them. Since disconnect() also doesn’t close connected client sockets, threads can leak across unload/shutdown.
Agent Prompt
## Issue description
Client handler threads can block indefinitely on `recv()` and never exit during `disconnect()`, leaking threads and sockets.
## Issue Context
`disconnect()` currently only closes the listening socket; client sockets are not closed, and client sockets are configured for blocking reads.
## Fix Focus Areas
- AbletonMCP_Remote_Script/__init__.py[210-227]
- AbletonMCP_Remote_Script/__init__.py[286-299]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (1)
MCP_Server/tools/transport_tools.py (1)
71-79: Add fail-fast validation for documented parameter bounds.These tools document constraints (e.g., tempo/time-signature/loop fields) but currently pass values through unchecked. Lightweight validation here will reduce avoidable remote command failures.
Also applies to: 127-137, 162-173
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCP_Server/tools/transport_tools.py` around lines 71 - 79, The functions (e.g., async def set_tempo) currently accept parameters that are documented to be bounded but do not validate them before calling get_connection/send_command; add lightweight fail-fast checks in set_tempo to validate tempo is a float and within 20.0–999.0 and raise a ValueError (or return an error string) if out of range, and apply the same pattern to the other tools that document bounds (e.g., set_time_signature and set_loop): validate input types and documented ranges up front, return/raise a clear error immediately, and only call conn.send_command when inputs pass validation so remote commands aren’t invoked with invalid parameters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@install.sh`:
- Around line 59-74: Validate the user selection before indexing DIRS: check
that choice is a positive integer (e.g., match against ^[0-9]+$) and that it
falls between 1 and ${`#DIRS`[@]}; if the check fails, print a clear error and
re-prompt or exit with a non-zero status instead of computing idx/CHOSEN. Only
compute idx=$((choice - 1)) and set CHOSEN="${DIRS[$idx]}" after the validation
succeeds so TARGET="$CHOSEN/AbletonMCP" cannot become "/AbletonMCP" due to
invalid input; keep existing symbols (choice, DIRS, idx, CHOSEN, TARGET,
REMOTE_SCRIPT_SRC) so the rest of the flow is unchanged.
- Around line 36-38: The find invocation inside the while loop that populates
DIRS uses -maxdepth 2 which is too shallow for Ableton app bundles; update the
find command (the line feeding the while loop that populates DIRS) to use a
greater maxdepth (e.g., -maxdepth 5) so paths like /Applications/Ableton Live
<version>.app/Contents/App-Resources/MIDI Remote Scripts are discovered
correctly; keep the same -type d -name "MIDI Remote Scripts" -path "*/Ableton*"
and preserve the null-separated output and stderr redirection.
In `@MCP_Server/cache.py`:
- Around line 26-28: The set method currently treats ttl with truthiness so an
explicit ttl=0 is ignored; update MCP_Server.cache.Cache.set (the set method
that writes to self._cache using self.default_ttl) to check "if ttl is None" (or
use a conditional expression testing is None) and only fall back to
self.default_ttl when ttl is None, ensuring ttl=0 is preserved as a valid
expiry; keep the same tuple storage (value, expiry_time) and TTL arithmetic
using time.time() + ttl.
In `@MCP_Server/connection.py`:
- Around line 59-83: ensure_connected currently performs connect() and the
subsequent validation without holding self._lock, allowing two coroutines (via
get_connection) to race and overwrite shared self.reader/self.writer; fix by
serializing reconnects: acquire self._lock at the start of ensure_connected (or
wrap the connect(), _send_and_receive("get_session_info") validation, and
disconnect() calls inside a with await self._lock) so only one coroutine can
perform connect/validation/disconnect at a time. Apply the same protection to
the other reconnect path referenced around the get_connection usage (the logic
at the other occurrence noted in the comment) so both code paths use self._lock
when touching reader/writer or calling connect/disconnect.
- Around line 110-129: send_batch() currently treats a non-list server response
as a single successful result and returns [result], which drops all other
commands; fix by detecting the non-list (batch-unsupported) case, decoding the
original payload (the JSON list of commands) and replaying each command
sequentially: for each command in the decoded payload write it to self.writer,
await drain, call self._read_json_response() to parse that single response, and
append either {"error": message} or the result to the results list; return the
full results list so callers receive per-command results/errors. Ensure you
reference send_batch, payload, self.writer, and self._read_json_response in your
change.
In `@MCP_Server/server.py`:
- Around line 22-23: The session caches use exact keys ("session_info",
"full_session_state", "all_tracks_info") while browser_tools.py only calls
cache.invalidate("browser") and cache.invalidate("track"), so those mutations
don't clear session entries; update browser_tools.py to call
cache.invalidate_all() (or at least include session-prefixed keys) after
mutation points such as load_instrument_or_effect and load_drum_kit so the
global ResponseCache (cache) is fully cleared for affected data; specifically
replace or augment calls to
cache.invalidate("browser")/cache.invalidate("track") with
cache.invalidate_all() where devices are added/changed.
In `@MCP_Server/tools/ai_tools.py`:
- Around line 603-621: The mode inference in generate_bassline() incorrectly
treats many scale types as major because it only checks a small minor whitelist;
replace this fragile heuristic by mapping scale_type to mode explicitly (e.g.,
add a SCALE_TO_MODE dict and use mode = SCALE_TO_MODE.get(scale_type, "major")
or accept an explicit mode parameter) and update any logic that uses mode (the
existing mode variable and downstream Roman-numeral resolution) so scales like
"blues" or "locrian" resolve to the correct "minor" or "dorian"/"locrian" modes
rather than defaulting to major; ensure SCALE_INTERVALS lookups remain unchanged
but use the new mapping to determine major/minor behavior.
- Around line 433-459: In generate_chord_progression, validate the mode
parameter before any parsing: ensure mode is exactly "major" or "minor" (or
explicitly accept agreed synonyms) and raise a clear ValueError if it's invalid
so _parse_roman_numeral isn't fed a wrong mode; update the top of the function
(before resolving presets and calling _parse_roman_numeral) to check the mode
string and normalize it if needed, referencing the function name
generate_chord_progression and the parameter mode.
- Around line 523-571: The bug is inconsistent time-unit math: compute a beat
length in quarter-note units using beat_unit (e.g., beat_length_quarters = 4.0 /
beat_unit) and then use that to derive step_duration, bar_offset, and
total_beats so all timings use the same unit; specifically update step_duration,
bar_offset (inside the bars loop) and the returned total_beats to use
beat_length_quarters (with steps_per_beat = 16 / beat_unit if needed) so the
grid from patterns returned by _get_drum_pattern and the notes'
start_time/duration are correct for non-*/4 meters.
In `@MCP_Server/tools/browser_tools.py`:
- Around line 143-148: The load_browser_item handler that calls get_connection()
and conn.send_command("load_browser_item", ...) currently only invalidates
"browser" and "track"; update this to also invalidate session/device-related
caches so device chain changes aren't missed—specifically call cache.invalidate
for "full_session_state", "all_tracks_info", and any per-device keys (e.g.,
"device_{device_id}" or pattern "device_*") after the send_command; apply the
same additional invalidations to the other similar block around the 166-205
range that performs load_browser_item operations.
In `@MCP_Server/tools/clip_tools.py`:
- Around line 178-205: The default pitch span in remove_notes_from_clip is off
by one: change the default parameter pitch_span from 127 to 128 so the function
uses the full MIDI range when callers rely on the default; update the function
signature for remove_notes_from_clip to pitch_span: int = 128 and ensure any
docstring wording that mentions default pitch span remains accurate.
In `@MCP_Server/tools/track_tools.py`:
- Around line 20-23: The current exception handling in functions like the MIDI
track handler (the except Exception as e block that calls logger.error and
returns a plain string) is inconsistent with success responses that use
json.dumps; create a small helper (e.g., wrap_tool_call or handle_tool_error)
that executes the tool logic, catches exceptions, logs the full exception via
logger.exception or logger.error(..., exc_info=True), and returns a consistent
JSON error payload using json.dumps({"success": False, "error": str(e)}) (or
your project's standard error schema); replace the repeated except blocks (the
shown logger.error/return lines and the ~10 other occurrences) to call this
helper so all handlers return structured JSON errors and logging includes
exception details.
In `@MCP_Server/tools/transport_tools.py`:
- Around line 20-23: The current bare except in transport_tools.py (in the
playback startup block, e.g., the except in the function that starts playback)
should be replaced with structured JSON error responses and full traceback
logging: change logger.error(...) to logger.exception(...) to capture the stack
trace and return json.dumps({"error": str(e)}) instead of a plain string; apply
the same pattern to other handlers in transport_tools (any functions that
currently use bare "except Exception as e" and return plain strings) to match
ai_tools' error format and ensure consistent parseable client-side responses.
In `@README.md`:
- Around line 104-108: Update the two plain fenced code blocks in README.md to
include a language tag of "text" to satisfy markdownlint MD040; specifically
modify the blocks containing the ASCII diagram ("AI Assistant <──MCP──> MCP
Server...") and the project tree block (starting with "MCP_Server/") so their
opening fences are ```text instead of ```; this will silence the lint warnings
without changing the block contents.
In `@ROADMAP.md`:
- Around line 3-5: Corrige la ortografía y acentuación en las líneas de resumen
clave: reemplaza "conexion" por "conexión", "cache" por "caché", "basica" por
"básica" en la línea que empieza "Estado actual: **67 herramientas**, conexion
async, cache, AI music theory basica." y cambia "item" por "ítem",
"investigacion" por "investigación" y "tecnica" por "técnica" en la frase que
empieza "Cada item tiene un veredicto..."; además, usa "reguetón" en lugar de
"regueton" si aparece en la lista de géneros y revisa otras ocurrencias
similares en el documento para aplicar las mismas correcciones.
---
Nitpick comments:
In `@MCP_Server/tools/transport_tools.py`:
- Around line 71-79: The functions (e.g., async def set_tempo) currently accept
parameters that are documented to be bounded but do not validate them before
calling get_connection/send_command; add lightweight fail-fast checks in
set_tempo to validate tempo is a float and within 20.0–999.0 and raise a
ValueError (or return an error string) if out of range, and apply the same
pattern to the other tools that document bounds (e.g., set_time_signature and
set_loop): validate input types and documented ranges up front, return/raise a
clear error immediately, and only call conn.send_command when inputs pass
validation so remote commands aren’t invoked with invalid parameters.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5e683390-eb2d-4e40-9afc-869e66e944d1
📒 Files selected for processing (18)
AbletonMCP_Remote_Script/__init__.pyCLAUDE.mdMCP_Server/__init__.pyMCP_Server/cache.pyMCP_Server/connection.pyMCP_Server/server.pyMCP_Server/tools/__init__.pyMCP_Server/tools/ai_tools.pyMCP_Server/tools/browser_tools.pyMCP_Server/tools/clip_tools.pyMCP_Server/tools/device_tools.pyMCP_Server/tools/scene_tools.pyMCP_Server/tools/session_tools.pyMCP_Server/tools/track_tools.pyMCP_Server/tools/transport_tools.pyREADME.mdROADMAP.mdinstall.sh
| while IFS= read -r -d '' dir; do | ||
| DIRS+=("$dir") | ||
| done < <(find /Applications -maxdepth 2 -type d -name "MIDI Remote Scripts" -path "*/Ableton*" -print0 2>/dev/null) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, examine the install.sh file around the mentioned lines
fd install.sh -type fRepository: ahujasid/ableton-mcp
Length of output: 234
🏁 Script executed:
# Correct fd syntax
fd install.sh -t fRepository: ahujasid/ableton-mcp
Length of output: 74
🏁 Script executed:
# Read install.sh with line numbers
cat -n install.shRepository: ahujasid/ableton-mcp
Length of output: 3521
🌐 Web query:
Ableton Live app bundle MIDI Remote Scripts directory path macOS
💡 Result:
On macOS, the built‑in (app-bundle) MIDI Remote Scripts folder is inside the Live app package:
/Applications/Ableton Live <version>.app/Contents/App-Resources/MIDI Remote Scripts/(you can also Finder → right‑click Ableton Live .app → Show Package Contents →Contents/App-Resources/MIDI Remote Scripts) [1][2]
If you meant the user locations Ableton documents:
- Remote Scripts (User Library):
~/Music/Ableton/User Library/Remote Scripts/[3] - User Remote Scripts:
~/Library/Preferences/Ableton/Live <version>/User Remote Scripts/[4]
Sources: [1] Remotify knowledge base, [2] third‑party install guides/GitHub instructions reflecting Live’s macOS bundle layout, [3] Ableton “Installing third-party remote scripts”, [4] Ableton “Creating your own Control Surface script.
Fix Ableton app-bundle discovery depth.
Line 38 uses -maxdepth 2, which is too shallow for standard Ableton .app bundle paths. The MIDI Remote Scripts directory is located at /Applications/Ableton Live <version>.app/Contents/App-Resources/MIDI Remote Scripts/, which is 4 levels deep. This causes false "No Ableton installation found" failures.
Proposed fix
-done < <(find /Applications -maxdepth 2 -type d -name "MIDI Remote Scripts" -path "*/Ableton*" -print0 2>/dev/null)
+done < <(find /Applications -maxdepth 4 -type d -name "MIDI Remote Scripts" -path "*/Ableton*" -print0 2>/dev/null)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| while IFS= read -r -d '' dir; do | |
| DIRS+=("$dir") | |
| done < <(find /Applications -maxdepth 2 -type d -name "MIDI Remote Scripts" -path "*/Ableton*" -print0 2>/dev/null) | |
| while IFS= read -r -d '' dir; do | |
| DIRS+=("$dir") | |
| done < <(find /Applications -maxdepth 4 -type d -name "MIDI Remote Scripts" -path "*/Ableton*" -print0 2>/dev/null) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@install.sh` around lines 36 - 38, The find invocation inside the while loop
that populates DIRS uses -maxdepth 2 which is too shallow for Ableton app
bundles; update the find command (the line feeding the while loop that populates
DIRS) to use a greater maxdepth (e.g., -maxdepth 5) so paths like
/Applications/Ableton Live <version>.app/Contents/App-Resources/MIDI Remote
Scripts are discovered correctly; keep the same -type d -name "MIDI Remote
Scripts" -path "*/Ableton*" and preserve the null-separated output and stderr
redirection.
| read -rp " Which one? [1-${#DIRS[@]}, or 'a' for all]: " choice | ||
| if [ "$choice" = "a" ]; then | ||
| for dir in "${DIRS[@]}"; do | ||
| TARGET="$dir/AbletonMCP" | ||
| mkdir -p "$TARGET" | ||
| cp "$REMOTE_SCRIPT_SRC" "$TARGET/__init__.py" | ||
| echo " Installed to: $TARGET" | ||
| done | ||
| echo "" | ||
| echo "=== Done! ===" | ||
| echo "Restart Ableton and select 'AbletonMCP' in Settings > Link, Tempo & MIDI > Control Surface." | ||
| exit 0 | ||
| else | ||
| idx=$((choice - 1)) | ||
| CHOSEN="${DIRS[$idx]}" | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the install.sh file
find . -name "install.sh" -type f | head -20Repository: ahujasid/ableton-mcp
Length of output: 76
🏁 Script executed:
# Get the full context of install.sh around lines 59-74
if git ls-files | grep -q "install.sh"; then
git ls-files | grep "install.sh" | head -5
fiRepository: ahujasid/ableton-mcp
Length of output: 74
🏁 Script executed:
# Try to find install.sh in the repository
fd install.shRepository: ahujasid/ableton-mcp
Length of output: 74
🏁 Script executed:
cat -n install.shRepository: ahujasid/ableton-mcp
Length of output: 3521
🏁 Script executed:
# Test bash arithmetic with non-numeric input
bash <<'EOF'
# Simulate the problematic code
DIRS=("dir1" "dir2")
choice="abc"
idx=$((choice - 1))
echo "choice='abc' → idx=$idx"
choice="0"
idx=$((choice - 1))
echo "choice='0' → idx=$idx"
choice="5"
idx=$((choice - 1))
echo "choice='5' → idx=$idx, array length=${`#DIRS`[@]}"
CHOSEN="${DIRS[$idx]}"
echo "CHOSEN='$CHOSEN' (empty if out of range)"
# Show what happens with empty CHOSEN
TARGET="$CHOSEN/AbletonMCP"
echo "TARGET='$TARGET' (dangerous if empty!)"
EOFRepository: ahujasid/ableton-mcp
Length of output: 217
Validate selection input before indexing DIRS.
Lines 72–74 accept arbitrary input and index directly without validation. Non-numeric input (e.g., "abc") or out-of-range values leave CHOSEN empty, causing TARGET to become /AbletonMCP—attempting installation at the filesystem root instead of the intended directory.
Proposed fix
else
- idx=$((choice - 1))
- CHOSEN="${DIRS[$idx]}"
+ if ! [[ "$choice" =~ ^[0-9]+$ ]]; then
+ echo " ERROR: Invalid selection: $choice"
+ exit 1
+ fi
+ idx=$((choice - 1))
+ if [ "$idx" -lt 0 ] || [ "$idx" -ge "${`#DIRS`[@]}" ]; then
+ echo " ERROR: Selection out of range: $choice"
+ exit 1
+ fi
+ CHOSEN="${DIRS[$idx]}"
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| read -rp " Which one? [1-${#DIRS[@]}, or 'a' for all]: " choice | |
| if [ "$choice" = "a" ]; then | |
| for dir in "${DIRS[@]}"; do | |
| TARGET="$dir/AbletonMCP" | |
| mkdir -p "$TARGET" | |
| cp "$REMOTE_SCRIPT_SRC" "$TARGET/__init__.py" | |
| echo " Installed to: $TARGET" | |
| done | |
| echo "" | |
| echo "=== Done! ===" | |
| echo "Restart Ableton and select 'AbletonMCP' in Settings > Link, Tempo & MIDI > Control Surface." | |
| exit 0 | |
| else | |
| idx=$((choice - 1)) | |
| CHOSEN="${DIRS[$idx]}" | |
| fi | |
| read -rp " Which one? [1-${`#DIRS`[@]}, or 'a' for all]: " choice | |
| if [ "$choice" = "a" ]; then | |
| for dir in "${DIRS[@]}"; do | |
| TARGET="$dir/AbletonMCP" | |
| mkdir -p "$TARGET" | |
| cp "$REMOTE_SCRIPT_SRC" "$TARGET/__init__.py" | |
| echo " Installed to: $TARGET" | |
| done | |
| echo "" | |
| echo "=== Done! ===" | |
| echo "Restart Ableton and select 'AbletonMCP' in Settings > Link, Tempo & MIDI > Control Surface." | |
| exit 0 | |
| else | |
| if ! [[ "$choice" =~ ^[0-9]+$ ]]; then | |
| echo " ERROR: Invalid selection: $choice" | |
| exit 1 | |
| fi | |
| idx=$((choice - 1)) | |
| if [ "$idx" -lt 0 ] || [ "$idx" -ge "${`#DIRS`[@]}" ]; then | |
| echo " ERROR: Selection out of range: $choice" | |
| exit 1 | |
| fi | |
| CHOSEN="${DIRS[$idx]}" | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@install.sh` around lines 59 - 74, Validate the user selection before indexing
DIRS: check that choice is a positive integer (e.g., match against ^[0-9]+$) and
that it falls between 1 and ${`#DIRS`[@]}; if the check fails, print a clear error
and re-prompt or exit with a non-zero status instead of computing idx/CHOSEN.
Only compute idx=$((choice - 1)) and set CHOSEN="${DIRS[$idx]}" after the
validation succeeds so TARGET="$CHOSEN/AbletonMCP" cannot become "/AbletonMCP"
due to invalid input; keep existing symbols (choice, DIRS, idx, CHOSEN, TARGET,
REMOTE_SCRIPT_SRC) so the rest of the flow is unchanged.
| def set(self, key: str, value: Any, ttl: Optional[float] = None): | ||
| """Cache a value with optional custom TTL""" | ||
| self._cache[key] = (value, time.time() + (ttl or self.default_ttl)) |
There was a problem hiding this comment.
Preserve explicit ttl=0 semantics.
Line 28 uses truthiness (ttl or ...), so an explicit ttl=0 is ignored. Use is None to distinguish omitted TTL from zero.
Proposed fix
- self._cache[key] = (value, time.time() + (ttl or self.default_ttl))
+ ttl_value = self.default_ttl if ttl is None else ttl
+ self._cache[key] = (value, time.time() + ttl_value)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MCP_Server/cache.py` around lines 26 - 28, The set method currently treats
ttl with truthiness so an explicit ttl=0 is ignored; update
MCP_Server.cache.Cache.set (the set method that writes to self._cache using
self.default_ttl) to check "if ttl is None" (or use a conditional expression
testing is None) and only fall back to self.default_ttl when ttl is None,
ensuring ttl=0 is preserved as a valid expiry; keep the same tuple storage
(value, expiry_time) and TTL arithmetic using time.time() + ttl.
| async def ensure_connected(self): | ||
| """Ensure we have an active connection, reconnecting if needed""" | ||
| if self._connected and self.writer and not self.writer.is_closing(): | ||
| return | ||
|
|
||
| self._connected = False | ||
| max_attempts = 3 | ||
| for attempt in range(1, max_attempts + 1): | ||
| logger.info(f"Connecting to Ableton (attempt {attempt}/{max_attempts})...") | ||
| if await self.connect(): | ||
| # Validate with a quick command | ||
| try: | ||
| await self._send_and_receive("get_session_info") | ||
| logger.info("Connection validated successfully") | ||
| return | ||
| except Exception as e: | ||
| logger.warning(f"Connection validation failed: {e}") | ||
| await self.disconnect() | ||
|
|
||
| if attempt < max_attempts: | ||
| await asyncio.sleep(0.5) | ||
|
|
||
| raise ConnectionError( | ||
| "Could not connect to Ableton. Make sure the Remote Script is running." | ||
| ) |
There was a problem hiding this comment.
Serialize reconnects around ensure_connected().
get_connection() awaits ensure_connected() outside self._lock, so two concurrent tool calls during a reconnect can run connect() and the validation request in parallel against the same shared streams. One coroutine can overwrite self.reader/self.writer while the other is reading, which is a real race.
Also applies to: 201-206
🧰 Tools
🪛 Ruff (0.15.6)
[warning] 74-74: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MCP_Server/connection.py` around lines 59 - 83, ensure_connected currently
performs connect() and the subsequent validation without holding self._lock,
allowing two coroutines (via get_connection) to race and overwrite shared
self.reader/self.writer; fix by serializing reconnects: acquire self._lock at
the start of ensure_connected (or wrap the connect(),
_send_and_receive("get_session_info") validation, and disconnect() calls inside
a with await self._lock) so only one coroutine can perform
connect/validation/disconnect at a time. Apply the same protection to the other
reconnect path referenced around the get_connection usage (the logic at the
other occurrence noted in the comment) so both code paths use self._lock when
touching reader/writer or calling connect/disconnect.
| try: | ||
| self.writer.write(payload) | ||
| await self.writer.drain() | ||
|
|
||
| response_data = await self._read_json_response() | ||
| responses = json.loads(response_data.decode("utf-8")) | ||
|
|
||
| if isinstance(responses, list): | ||
| results = [] | ||
| for resp in responses: | ||
| if resp.get("status") == "error": | ||
| results.append( | ||
| {"error": resp.get("message", "Unknown error")} | ||
| ) | ||
| else: | ||
| results.append(resp.get("result", {})) | ||
| return results | ||
| else: | ||
| # Server returned single response — batch not supported | ||
| return [responses.get("result", {})] |
There was a problem hiding this comment.
send_batch() drops commands when batch mode is unsupported.
The non-list branch returns a one-element result list instead of replaying the original commands sequentially, so callers lose both the error and every command after the first.
📦 Suggested fix
if isinstance(responses, list):
results = []
for resp in responses:
if resp.get("status") == "error":
results.append(
{"error": resp.get("message", "Unknown error")}
)
else:
results.append(resp.get("result", {}))
return results
- else:
- # Server returned single response — batch not supported
- return [responses.get("result", {})]
+
+ # Server returned a single response — batch not supported.
+ results = []
+ for cmd in commands:
+ try:
+ results.append(
+ await self._send_and_receive(
+ cmd["type"],
+ cmd.get("params", {}),
+ )
+ )
+ except Exception as e:
+ results.append({"error": str(e)})
+ return results🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MCP_Server/connection.py` around lines 110 - 129, send_batch() currently
treats a non-list server response as a single successful result and returns
[result], which drops all other commands; fix by detecting the non-list
(batch-unsupported) case, decoding the original payload (the JSON list of
commands) and replaying each command sequentially: for each command in the
decoded payload write it to self.writer, await drain, call
self._read_json_response() to parse that single response, and append either
{"error": message} or the result to the results list; return the full results
list so callers receive per-command results/errors. Ensure you reference
send_batch, payload, self.writer, and self._read_json_response in your change.
| async def remove_notes_from_clip( | ||
| track_index: int, | ||
| clip_index: int, | ||
| from_time: float = 0.0, | ||
| time_span: float = 999.0, | ||
| from_pitch: int = 0, | ||
| pitch_span: int = 127, | ||
| ) -> str: | ||
| """Remove notes from a clip within the specified range. | ||
|
|
||
| Args: | ||
| track_index: The zero-based index of the track. | ||
| clip_index: The zero-based index of the clip slot. | ||
| from_time: Start of the time range in beats (default 0.0). | ||
| time_span: Length of the time range in beats (default 999.0). | ||
| from_pitch: Lowest MIDI pitch to remove (default 0). | ||
| pitch_span: Number of pitches above from_pitch to include (default 127). | ||
| """ | ||
| try: | ||
| conn = await get_connection() | ||
| result = await conn.send_command("remove_notes_from_clip", { | ||
| "track_index": track_index, | ||
| "clip_index": clip_index, | ||
| "from_time": from_time, | ||
| "time_span": time_span, | ||
| "from_pitch": from_pitch, | ||
| "pitch_span": pitch_span, | ||
| }) |
There was a problem hiding this comment.
Use the full MIDI pitch span for the default note-removal range.
The Remote Script handler defaults pitch_span to 128. With 127 here, a default “remove everything” call leaves note 127 behind if the span is interpreted as a count, which is how the server-side API is defined.
🎹 Suggested fix
async def remove_notes_from_clip(
track_index: int,
clip_index: int,
from_time: float = 0.0,
time_span: float = 999.0,
from_pitch: int = 0,
- pitch_span: int = 127,
+ pitch_span: int = 128,
) -> str:
@@
- pitch_span: Number of pitches above from_pitch to include (default 127).
+ pitch_span: Number of pitches above from_pitch to include (default 128).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MCP_Server/tools/clip_tools.py` around lines 178 - 205, The default pitch
span in remove_notes_from_clip is off by one: change the default parameter
pitch_span from 127 to 128 so the function uses the full MIDI range when callers
rely on the default; update the function signature for remove_notes_from_clip to
pitch_span: int = 128 and ensure any docstring wording that mentions default
pitch span remains accurate.
| except Exception as e: | ||
| logger.error(f"Error creating MIDI track: {e}") | ||
| return f"Error creating MIDI track: {e}" | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and read the track_tools.py file
find . -name "track_tools.py" -type fRepository: ahujasid/ableton-mcp
Length of output: 97
🏁 Script executed:
# Read the specific file around lines 20-23
cat -n MCP_Server/tools/track_tools.py | head -50Repository: ahujasid/ableton-mcp
Length of output: 2316
🏁 Script executed:
# Count all "except Exception" patterns in the file
rg "except Exception" MCP_Server/tools/track_tools.py -A 2 -B 2Repository: ahujasid/ableton-mcp
Length of output: 2564
Consolidate error handling in tool handlers to return structured error responses.
This pattern (repeated 11 times throughout the file) returns inconsistent error formats: success responses use json.dumps(), but errors return plain strings. This breaks the error contract for downstream handling. Replace with a helper wrapper that returns structured JSON errors consistently.
Refactor direction
+async def _run_write_command(get_connection, cache, command: str, payload: dict, err_ctx: str) -> str:
+ try:
+ conn = await get_connection()
+ result = await conn.send_command(command, payload)
+ cache.invalidate_all()
+ return json.dumps(result, indent=2)
+ except Exception:
+ logger.exception(err_ctx)
+ return json.dumps({"status": "error", "message": err_ctx}, indent=2)- try:
- conn = await get_connection()
- result = await conn.send_command("set_track_name", {"track_index": track_index, "name": name})
- cache.invalidate_all()
- return json.dumps(result, indent=2)
- except Exception as e:
- logger.error(f"Error setting track name: {e}")
- return f"Error setting track name: {e}"
+ return await _run_write_command(
+ get_connection,
+ cache,
+ "set_track_name",
+ {"track_index": track_index, "name": name},
+ "Error setting track name",
+ )🧰 Tools
🪛 Ruff (0.15.6)
[warning] 20-20: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MCP_Server/tools/track_tools.py` around lines 20 - 23, The current exception
handling in functions like the MIDI track handler (the except Exception as e
block that calls logger.error and returns a plain string) is inconsistent with
success responses that use json.dumps; create a small helper (e.g.,
wrap_tool_call or handle_tool_error) that executes the tool logic, catches
exceptions, logs the full exception via logger.exception or logger.error(...,
exc_info=True), and returns a consistent JSON error payload using
json.dumps({"success": False, "error": str(e)}) (or your project's standard
error schema); replace the repeated except blocks (the shown logger.error/return
lines and the ~10 other occurrences) to call this helper so all handlers return
structured JSON errors and logging includes exception details.
| except Exception as e: | ||
| logger.error(f"Error starting playback: {e}") | ||
| return f"Error starting playback: {e}" | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the transport_tools.py file around lines 20-23
fd -t f transport_tools.pyRepository: ahujasid/ableton-mcp
Length of output: 99
🏁 Script executed:
# Get the file structure to find transport_tools.py
find . -name "transport_tools.py" -type f 2>/dev/null | head -20Repository: ahujasid/ableton-mcp
Length of output: 101
🏁 Script executed:
# List files in MCP_Server/tools directory
ls -la MCP_Server/tools/ 2>/dev/null || echo "Directory not found, searching for it..."
find . -path "*/MCP_Server/tools/*" -type f 2>/dev/null | head -20Repository: ahujasid/ableton-mcp
Length of output: 1073
🏁 Script executed:
cat -n MCP_Server/tools/transport_tools.py | head -40Repository: ahujasid/ableton-mcp
Length of output: 1651
🏁 Script executed:
# Check the full structure to understand error handling patterns
wc -l MCP_Server/tools/transport_tools.py
cat -n MCP_Server/tools/transport_tools.pyRepository: ahujasid/ableton-mcp
Length of output: 10683
🏁 Script executed:
# Search for similar exception handling patterns across all tool modules
rg "except Exception as e:" MCP_Server/tools/ -A 2Repository: ahujasid/ableton-mcp
Length of output: 18021
🏁 Script executed:
# Search for MCP error handling or exception patterns
rg -i "mcp.*error|exception.*mcp" --type pyRepository: ahujasid/ableton-mcp
Length of output: 179
🏁 Script executed:
# Check imports and MCP usage to understand expected error patterns
rg "from mcp|import mcp" MCP_Server/ -A 3Repository: ahujasid/ableton-mcp
Length of output: 2060
🏁 Script executed:
# Look for any error handling utilities or conventions
fd -t f "error|exception" MCP_Server/ --type py | head -10
rg "class.*Error|def.*error" MCP_Server/ --type py | head -20Repository: ahujasid/ableton-mcp
Length of output: 233
🏁 Script executed:
# Check ai_tools.py more closely to understand why it differs
cat -n MCP_Server/tools/ai_tools.py | grep -A 2 -B 2 "except Exception"Repository: ahujasid/ableton-mcp
Length of output: 2341
🏁 Script executed:
# Check if logger.exception is used anywhere in the codebase
rg "logger\.exception" MCP_Server/ --type pyRepository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Look for any MCP error handling patterns or examples
rg "raise|McpError|exception" MCP_Server/server.py -A 2 -B 2Repository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Check FastMCP documentation or error handling patterns in imports
head -100 MCP_Server/server.pyRepository: ahujasid/ableton-mcp
Length of output: 2481
Replace broad exception handling with structured error responses to match the pattern already used in ai_tools.
This error handling pattern (bare except Exception returning plain strings) is inconsistent across the tool modules. ai_tools.py already demonstrates the better approach by returning structured JSON errors with json.dumps({"error": str(e)}). Adopt this pattern throughout transport_tools and other handlers for consistent, parseable client-side error handling. Additionally, consider using logger.exception() instead of logger.error() to capture full stack traces for debugging.
🧰 Tools
🪛 Ruff (0.15.6)
[warning] 20-20: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MCP_Server/tools/transport_tools.py` around lines 20 - 23, The current bare
except in transport_tools.py (in the playback startup block, e.g., the except in
the function that starts playback) should be replaced with structured JSON error
responses and full traceback logging: change logger.error(...) to
logger.exception(...) to capture the stack trace and return json.dumps({"error":
str(e)}) instead of a plain string; apply the same pattern to other handlers in
transport_tools (any functions that currently use bare "except Exception as e"
and return plain strings) to match ai_tools' error format and ensure consistent
parseable client-side responses.
| ``` | ||
| AI Assistant <──MCP──> MCP Server (async Python) <──TCP:9877──> Ableton Remote Script | ||
| 67 tools, response cache 61 command handlers | ||
| AI music theory engine thread-safe execution | ||
| ``` |
There was a problem hiding this comment.
Add language tags to the plain-text fences.
markdownlint is already flagging both of these blocks (MD040), so they will keep the docs lint noisy or failing if the rule is enforced. text/plaintext is enough here.
📝 Suggested fix
-```
+```text
AI Assistant <──MCP──> MCP Server (async Python) <──TCP:9877──> Ableton Remote Script
67 tools, response cache 61 command handlers
AI music theory engine thread-safe execution@@
- +text
MCP_Server/
server.py # Entry point
connection.py # Async TCP connection
cache.py # Response cache
tools/
session_tools.py # Session info
track_tools.py # Track management
clip_tools.py # Clip operations
scene_tools.py # Scene management
device_tools.py # Device parameters
transport_tools.py # Transport controls
browser_tools.py # Browser navigation
ai_tools.py # Music theory
AbletonMCP_Remote_Script/
init.py # Runs inside Ableton
Also applies to: 123-140
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 104-104: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 104 - 108, Update the two plain fenced code blocks in
README.md to include a language tag of "text" to satisfy markdownlint MD040;
specifically modify the blocks containing the ASCII diagram ("AI Assistant
<──MCP──> MCP Server...") and the project tree block (starting with
"MCP_Server/") so their opening fences are ```text instead of ```; this will
silence the lint warnings without changing the block contents.
| Estado actual: **67 herramientas**, conexion async, cache, AI music theory basica. | ||
|
|
||
| Cada item tiene un veredicto de viabilidad basado en investigacion de la API de Ableton, complejidad tecnica, y valor para el usuario. Cuando se implemente algo, se tacha de la lista. |
There was a problem hiding this comment.
Polish Spanish orthography in key summary lines.
There are minor language quality issues (accents/style) in the high-visibility intro and genre list (e.g., “conexión”, “caché”, “básica”, “ítem”, “investigación”, “técnica”, and preferred “reguetón”).
Also applies to: 14-14
🧰 Tools
🪛 LanguageTool
[grammar] ~3-~3: Oración con errores
Context: ... herramientas**, conexion async, cache, AI music theory basica. Cada item tiene u...
(QB_NEW_ES_OTHER_ERROR_IDS_REPLACEMENT_MULTITOKEN)
[grammar] ~3-~3: Oración con errores
Context: ...rramientas**, conexion async, cache, AI music theory basica. Cada item tiene un vere...
(QB_NEW_ES_OTHER_ERROR_IDS_REPLACEMENT_MULTITOKEN)
[grammar] ~3-~3: Oración con errores
Context: ...ntas**, conexion async, cache, AI music theory basica. Cada item tiene un veredicto d...
(QB_NEW_ES_OTHER_ERROR_IDS_REPLACEMENT_MULTITOKEN)
[grammar] ~3-~3: Oración con errores
Context: ... conexion async, cache, AI music theory basica. Cada item tiene un veredicto de viabi...
(QB_NEW_ES_OTHER_ERROR_IDS_REPLACEMENT_MULTITOKEN)
[grammar] ~5-~5: Cambia la palabra o signo.
Context: ...acion de la API de Ableton, complejidad tecnica, y valor para el usuario. Cuando se impl...
(QB_NEW_ES_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ROADMAP.md` around lines 3 - 5, Corrige la ortografía y acentuación en las
líneas de resumen clave: reemplaza "conexion" por "conexión", "cache" por
"caché", "basica" por "básica" en la línea que empieza "Estado actual: **67
herramientas**, conexion async, cache, AI music theory basica." y cambia "item"
por "ítem", "investigacion" por "investigación" y "tecnica" por "técnica" en la
frase que empieza "Cada item tiene un veredicto..."; además, usa "reguetón" en
lugar de "regueton" si aparece en la lista de géneros y revisa otras ocurrencias
similares en el documento para aplicar las mismas correcciones.
Summary
Adds a researched roadmap based on 4 parallel investigation agents that analyzed:
Each item has a feasibility verdict and effort estimate.
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores