From afea349d67805662e6dc9226ea4ea87a351eb28f Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Mon, 16 Jun 2025 13:51:57 +0200 Subject: [PATCH] CI, added some memories --- .github/workflows/pytest.yml | 7 +-- .../deadlock_analysis_mcp_symbolic_tools.md | 63 +++++++++++++++++++ .serena/memories/suggested_commands.md | 18 +++++- pyproject.toml | 9 +-- 4 files changed, 85 insertions(+), 12 deletions(-) create mode 100644 .serena/memories/deadlock_analysis_mcp_symbolic_tools.md diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 6368dc2..64dd29f 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -55,9 +55,4 @@ jobs: run: uv pip install -e ".[dev]" - name: Test with pytest shell: bash - run: | - if [ "${{ runner.os }}" = "Linux" ]; then - uv run pytest -vv -m "not java and not rust" - else - uv run pytest -vv - fi + run: uv run poe test \ No newline at end of file diff --git a/.serena/memories/deadlock_analysis_mcp_symbolic_tools.md b/.serena/memories/deadlock_analysis_mcp_symbolic_tools.md new file mode 100644 index 0000000..fa3fc20 --- /dev/null +++ b/.serena/memories/deadlock_analysis_mcp_symbolic_tools.md @@ -0,0 +1,63 @@ +# Deadlock Analysis: MCP Symbolic Tools Timeout Issue + +## Problem Summary +Intermittent deadlocks occur when using symbolic tools (particularly `find_symbol`) through MCP clients. Successful calls are followed by calls that hang for 240 seconds before timing out. Once a timeout occurs, the language server calls will no longer respond. + +## ROOT CAUSE DISCOVERED: Asyncio Event Loop Interference + +### Critical Evidence from MCP Server Logs +The smoking gun was found in the MCP server logs showing **unawaited coroutines**: + +``` +INFO mcp.server.lowlevel.server:_handle_message:524 - Warning: RuntimeWarning: coroutine 'LanguageServer.request_full_symbol_tree' was never awaited +``` + +This warning is being logged by the **MCP server's `_handle_message` method**, not SerenaAgent code, indicating coroutines are being created in the MCP server's asyncio context but never properly awaited. + +### The Real Issue: Dual Asyncio Context Contamination + +**Architecture Problem:** +1. **MCP Server**: Runs its own asyncio event loop to handle incoming requests +2. **SerenaAgent**: Creates its own asyncio event loop in separate thread for language serve, and even a second loop for the dashboard if so configured +3. **Conflict**: When MCP server calls SerenaAgent tools, we have **two or three asyncio contexts interacting** + +**Deadlock Mechanism:** +``` +MCP Server (asyncio loop A) + → handles tool request + → calls SerenaAgent.find_symbol() + → SerenaAgent uses asyncio.run_coroutine_threadsafe() + → Creates coroutine in language server loop (loop B) + → BUT: Coroutine gets leaked into MCP's context and never awaited + → Dangling coroutines accumulate in MCP server's loop + → Eventually causes resource exhaustion/event loop blocking + → Language server appears to "hang" but it's actually MCP loop contamination +``` + +### Discarded Hypotheses +- **Not a timeout issue**: Language server actually works fine in isolation +- **Not abandoned threads**: The threading mechanism works correctly +- **Not LSP deadlock**: The TypeScript language server itself isn't hanging + +The actual issue is **asyncio context bleeding** between MCP server and SerenaAgent. + +### Why We Can't Reproduce Outside MCP +- **Direct script calls**: No MCP server, no dual asyncio context +- **Single event loop**: Only SerenaAgent's language server loop exists +- **No async interference**: Clean, isolated execution +- **MCP-specific**: Requires the exact async context interaction pattern + +## Technical Details + +### Key Files and Locations +- `src/multilspy/language_server.py:1870` - `run_coroutine_threadsafe` creates coroutines that leak to MCP context +- MCP server `_handle_message` - Where unawaited coroutine warnings appear + +## Solution +**Process Isolation** is the fundamental fix: +1. **Separate processes**: MCP server, SerenaAgent and Dashboard in different processes +2. **IPC communication**: Replace direct method calls with inter-process communication +3. **Clean async boundaries**: Each process manages its own asyncio context +4. **No coroutine leakage**: Complete isolation prevents context contamination + +This explains why the deadlock is **MCP-specific** and doesn't occur in direct tool execution - it's fundamentally about asyncio context contamination between the MCP server and SerenaAgent. diff --git a/.serena/memories/suggested_commands.md b/.serena/memories/suggested_commands.md index 920b4d0..9ce3d46 100644 --- a/.serena/memories/suggested_commands.md +++ b/.serena/memories/suggested_commands.md @@ -4,10 +4,24 @@ The following tasks should generally be executed using `uv run poe `. -- `lint`: This is the **only** allowed command for linting. Run as `uv run poe lint`. - `format`: This is the **only** allowed command for formatting. Run as `uv run poe format`. - `type-check`: This is the **only** allowed command for type checking. Run as `uv run poe type-check`. -- `test`: This is the preferred command for running tests (`uv run poe test [args]`). However, running tests directly with `uv run pytest [args]` is also permitted. +- `test`: This is the preferred command for running tests (`uv run poe test [args]`). You can select subsets of tests with markers, + the current markers are + ```toml + markers = [ + "python: language server running for Python", + "go: language server running for Go", + "java: language server running for Java", + "rust: language server running for Rust", + "typescript: language server running for TypeScript", + "php: language server running for PHP", + "snapshot: snapshot tests for symbolic editing operations", + "isolated_process: test runs with process isolated agent", + ] + ``` + By default, `uv run poe test` uses the markes set in the env var `PYTEST_MARKERS`, or, if it unset, uses `-m "not java and not rust and not isolated process"`. + You can override this behavior by simply passing the `-m` option to `uv run poe test`, e.g. `uv run poe test -m "python or go"`. For finishing a task, make sure format, type-check and test pass! Run them at the end of the task and if needed fix any issues that come up and run them again until they pass. \ No newline at end of file diff --git a/pyproject.toml b/pyproject.toml index 4132dd7..2fccdf2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -106,9 +106,10 @@ exclude = "^build/|^docs/" PYDEVD_DISABLE_FILE_VALIDATION = "1" [tool.poe.tasks] -# Uses PYTEST_MARKERS env var for default markers (defaults to "not java and not rust") -# Set PYTEST_MARKERS="" to run all tests -test = "pytest test --color=yes -m \"${PYTEST_MARKERS:-not java and not rust}\"" +# Uses PYTEST_MARKERS env var for default markers +# For custom markers, one can either adjust the env var or just use -m option in the command line, +# as the second -m option will override the first one. +test = "pytest test -vv -m \"${PYTEST_MARKERS:-not java and not rust and not isolated process}\"" _black_check = "black --check --exclude src/multilspy/ src scripts test" _ruff_check = "ruff check --exclude .venv/ --exclude src/multilspy/ src scripts test" _black_format = "black --exclude .venv/|src/multilspy/ src scripts test" @@ -259,6 +260,6 @@ markers = [ "rust: language server running for Rust", "typescript: language server running for TypeScript", "php: language server running for PHP", - "snapshot: snapshot tests", + "snapshot: snapshot tests for symbolic editing operations", "isolated_process: test runs with process isolated agent", ]