mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-11 03:13:51 +00:00
Fix X11/XCB crash when running MCP server in empty directories (#278)
Summary Fixes X11/XCB crash when running serena-mcp-server in directories without source files Adds headless environment detection to prevent GUI operations in SSH/WSL/Docker environments Improves error messages to provide clear guidance when no source files are found Problem When running serena-mcp-server --context ide-assistant --project /empty/dir in a headless environment (SSH, WSL, Docker), the application would crash with X11/XCB errors: [xcb] Unknown sequence number while appending request [xcb] You called XInitThreads, this is not your fault [xcb] Aborting, sorry about that. python: ../../src/xcb_io.c:157: append_pending_request: Assertion `\!xcb_xlib_unknown_seq_number' failed. Root Cause Empty directories cause ProjectConfig.autogenerate() to raise a ValueError The exception handler attempts to show a GUI error dialog using tkinter In headless environments, tkinter crashes with X11/XCB errors Solution Headless Detection: Added is_headless_environment() to detect when running without a display Skip GUI in Headless: Modified show_fatal_exception_safe() to skip GUI attempts in headless environments Better Error Messages: Improved the error message for empty projects with clear instructions
This commit is contained in:
1 parent
1c2492c63c
commit
f76ed700fa
6 files changed
+307
-6
No files matched your search
Vendored
-1
@@ -12,4 +12,3 @@
|
||||
"vibing"
|
||||
],
|
||||
}
|
||||
|
||||
@@ -87,9 +87,14 @@ class ProjectConfig(ToStringMixin):
|
||||
language_composition = determine_programming_language_composition(str(project_root))
|
||||
if len(language_composition) == 0:
|
||||
raise ValueError(
|
||||
f"Failed to autogenerate project.yaml: no programming language detected in project {project_root}. "
|
||||
f"You can either add some files that correspond to one of the supported programming languages, "
|
||||
f"or create the file {os.path.join(project_root, cls.rel_path_to_project_yml())} manually and specify the language there."
|
||||
f"No source files found in {project_root}\n\n"
|
||||
f"To use Serena with this project, you need to either:\n"
|
||||
f"1. Add source files in one of the supported languages (Python, JavaScript/TypeScript, Java, C#, Rust, Go, Ruby, C++, PHP)\n"
|
||||
f"2. Create a project configuration file manually at:\n"
|
||||
f" {os.path.join(project_root, cls.rel_path_to_project_yml())}\n\n"
|
||||
f"Example project.yml:\n"
|
||||
f" project_name: {project_name}\n"
|
||||
f" language: python # or typescript, java, csharp, rust, go, ruby, cpp, php\n"
|
||||
)
|
||||
# find the language with the highest percentage
|
||||
dominant_language = max(language_composition.keys(), key=lambda lang: language_composition[lang])
|
||||
|
||||
@@ -1,8 +1,45 @@
|
||||
import os
|
||||
import sys
|
||||
|
||||
from serena.agent import log
|
||||
|
||||
|
||||
def is_headless_environment() -> bool:
|
||||
"""
|
||||
Detect if we're running in a headless environment where GUI operations would fail.
|
||||
|
||||
Returns True if:
|
||||
- No DISPLAY variable on Linux/Unix
|
||||
- Running in SSH session
|
||||
- Running in WSL without X server
|
||||
- Running in Docker container
|
||||
"""
|
||||
# Check if we're on Windows - GUI usually works there
|
||||
if sys.platform == "win32":
|
||||
return False
|
||||
|
||||
# Check for DISPLAY variable (required for X11)
|
||||
if not os.environ.get("DISPLAY"):
|
||||
return True
|
||||
|
||||
# Check for SSH session
|
||||
if os.environ.get("SSH_CONNECTION") or os.environ.get("SSH_CLIENT"):
|
||||
return True
|
||||
|
||||
# Check for common CI/container environments
|
||||
if os.environ.get("CI") or os.environ.get("CONTAINER") or os.path.exists("/.dockerenv"):
|
||||
return True
|
||||
|
||||
# Check for WSL (only on Unix-like systems where os.uname exists)
|
||||
if hasattr(os, "uname"):
|
||||
if "microsoft" in os.uname().release.lower():
|
||||
# In WSL, even with DISPLAY set, X server might not be running
|
||||
# This is a simplified check - could be improved
|
||||
return True
|
||||
|
||||
return False
|
||||
|
||||
|
||||
def show_fatal_exception_safe(e: Exception) -> None:
|
||||
"""
|
||||
Shows the given exception in the GUI log viewer on the main thread and ensures that the exception is logged or at
|
||||
@@ -12,6 +49,11 @@ def show_fatal_exception_safe(e: Exception) -> None:
|
||||
log.error(f"Fatal exception: {e}", exc_info=e)
|
||||
print(f"Fatal exception: {e}", file=sys.stderr)
|
||||
|
||||
# Don't attempt GUI in headless environments
|
||||
if is_headless_environment():
|
||||
log.debug("Skipping GUI error display in headless environment")
|
||||
return
|
||||
|
||||
# attempt to show the error in the GUI
|
||||
try:
|
||||
# NOTE: The import can fail on macOS if Tk is not available (depends on Python interpreter installation, which uv
|
||||
@@ -19,5 +61,5 @@ def show_fatal_exception_safe(e: Exception) -> None:
|
||||
from serena.gui_log_viewer import show_fatal_exception
|
||||
|
||||
show_fatal_exception(e)
|
||||
except:
|
||||
pass
|
||||
except Exception as gui_error:
|
||||
log.debug(f"Failed to show GUI error dialog: {gui_error}")
|
||||
@@ -0,0 +1 @@
|
||||
# Empty init file for test package
|
||||
@@ -0,0 +1,138 @@
|
||||
import shutil
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from serena.config.serena_config import ProjectConfig
|
||||
from solidlsp.ls_config import Language
|
||||
|
||||
|
||||
class TestProjectConfigAutogenerate:
|
||||
"""Test class for ProjectConfig autogeneration functionality."""
|
||||
|
||||
def setup_method(self):
|
||||
"""Set up test environment before each test method."""
|
||||
# Create a temporary directory for testing
|
||||
self.test_dir = tempfile.mkdtemp()
|
||||
self.project_path = Path(self.test_dir)
|
||||
|
||||
def teardown_method(self):
|
||||
"""Clean up test environment after each test method."""
|
||||
# Remove the temporary directory
|
||||
shutil.rmtree(self.test_dir)
|
||||
|
||||
def test_autogenerate_empty_directory(self):
|
||||
"""Test that autogenerate raises ValueError with helpful message for empty directory."""
|
||||
with pytest.raises(ValueError) as exc_info:
|
||||
ProjectConfig.autogenerate(self.project_path, save_to_disk=False)
|
||||
|
||||
error_message = str(exc_info.value)
|
||||
# Check that the error message contains all the key information
|
||||
assert "No source files found" in error_message
|
||||
assert str(self.project_path.resolve()) in error_message
|
||||
assert "To use Serena with this project" in error_message
|
||||
assert "Add source files in one of the supported languages" in error_message
|
||||
assert "Create a project configuration file manually" in error_message
|
||||
assert str(Path(".serena") / "project.yml") in error_message
|
||||
assert "Example project.yml:" in error_message
|
||||
assert f"project_name: {self.project_path.name}" in error_message
|
||||
assert "language: python" in error_message
|
||||
|
||||
def test_autogenerate_with_python_files(self):
|
||||
"""Test successful autogeneration with Python source files."""
|
||||
# Create a Python file
|
||||
python_file = self.project_path / "main.py"
|
||||
python_file.write_text("def hello():\n print('Hello, world!')\n")
|
||||
|
||||
# Run autogenerate
|
||||
config = ProjectConfig.autogenerate(self.project_path, save_to_disk=False)
|
||||
|
||||
# Verify the configuration
|
||||
assert config.project_name == self.project_path.name
|
||||
assert config.language == Language.PYTHON
|
||||
|
||||
def test_autogenerate_with_multiple_languages(self):
|
||||
"""Test autogeneration picks dominant language when multiple are present."""
|
||||
# Create files for multiple languages
|
||||
(self.project_path / "main.py").write_text("print('Python')")
|
||||
(self.project_path / "util.py").write_text("def util(): pass")
|
||||
(self.project_path / "small.js").write_text("console.log('JS');")
|
||||
|
||||
# Run autogenerate - should pick Python as dominant
|
||||
config = ProjectConfig.autogenerate(self.project_path, save_to_disk=False)
|
||||
|
||||
assert config.language == Language.PYTHON
|
||||
|
||||
def test_autogenerate_saves_to_disk(self):
|
||||
"""Test that autogenerate can save the configuration to disk."""
|
||||
# Create a Go file
|
||||
go_file = self.project_path / "main.go"
|
||||
go_file.write_text("package main\n\nfunc main() {}\n")
|
||||
|
||||
# Run autogenerate with save_to_disk=True
|
||||
config = ProjectConfig.autogenerate(self.project_path, save_to_disk=True)
|
||||
|
||||
# Verify the configuration file was created
|
||||
config_path = self.project_path / ".serena" / "project.yml"
|
||||
assert config_path.exists()
|
||||
|
||||
# Verify the content
|
||||
assert config.language == Language.GO
|
||||
|
||||
def test_autogenerate_nonexistent_path(self):
|
||||
"""Test that autogenerate raises FileNotFoundError for non-existent path."""
|
||||
non_existent = self.project_path / "does_not_exist"
|
||||
|
||||
with pytest.raises(FileNotFoundError) as exc_info:
|
||||
ProjectConfig.autogenerate(non_existent, save_to_disk=False)
|
||||
|
||||
assert "Project root not found" in str(exc_info.value)
|
||||
|
||||
def test_autogenerate_with_gitignored_files_only(self):
|
||||
"""Test autogenerate behavior when only gitignored files exist."""
|
||||
# Create a .gitignore that ignores all Python files
|
||||
gitignore = self.project_path / ".gitignore"
|
||||
gitignore.write_text("*.py\n")
|
||||
|
||||
# Create Python files that will be ignored
|
||||
(self.project_path / "ignored.py").write_text("print('ignored')")
|
||||
|
||||
# Should still raise ValueError as no source files are detected
|
||||
with pytest.raises(ValueError) as exc_info:
|
||||
ProjectConfig.autogenerate(self.project_path, save_to_disk=False)
|
||||
|
||||
assert "No source files found" in str(exc_info.value)
|
||||
|
||||
def test_autogenerate_custom_project_name(self):
|
||||
"""Test autogenerate with custom project name."""
|
||||
# Create a TypeScript file
|
||||
ts_file = self.project_path / "index.ts"
|
||||
ts_file.write_text("const greeting: string = 'Hello';\n")
|
||||
|
||||
# Run autogenerate with custom name
|
||||
custom_name = "my-custom-project"
|
||||
config = ProjectConfig.autogenerate(self.project_path, project_name=custom_name, save_to_disk=False)
|
||||
|
||||
assert config.project_name == custom_name
|
||||
assert config.language == Language.TYPESCRIPT
|
||||
|
||||
def test_autogenerate_error_message_format(self):
|
||||
"""Test the specific format of the error message for better user experience."""
|
||||
with pytest.raises(ValueError) as exc_info:
|
||||
ProjectConfig.autogenerate(self.project_path, save_to_disk=False)
|
||||
|
||||
error_lines = str(exc_info.value).split("\n")
|
||||
|
||||
# Verify the structure of the error message
|
||||
assert len(error_lines) >= 8 # Should have multiple lines of helpful information
|
||||
|
||||
# Check for numbered instructions
|
||||
assert any("1." in line for line in error_lines)
|
||||
assert any("2." in line for line in error_lines)
|
||||
|
||||
# Check for supported languages list
|
||||
assert any("Python" in line and "TypeScript" in line for line in error_lines)
|
||||
|
||||
# Check example includes comment about language options
|
||||
assert any("# or typescript, java, csharp" in line for line in error_lines)
|
||||
@@ -0,0 +1,116 @@
|
||||
import os
|
||||
from unittest.mock import MagicMock, Mock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from serena.util.exception import is_headless_environment, show_fatal_exception_safe
|
||||
|
||||
|
||||
class TestHeadlessEnvironmentDetection:
|
||||
"""Test class for headless environment detection functionality."""
|
||||
|
||||
def test_is_headless_no_display(self):
|
||||
"""Test that environment without DISPLAY is detected as headless on Linux."""
|
||||
with patch("sys.platform", "linux"):
|
||||
with patch.dict(os.environ, {}, clear=True):
|
||||
assert is_headless_environment() is True
|
||||
|
||||
def test_is_headless_ssh_connection(self):
|
||||
"""Test that SSH sessions are detected as headless."""
|
||||
with patch("sys.platform", "linux"):
|
||||
with patch.dict(os.environ, {"SSH_CONNECTION": "192.168.1.1 22 192.168.1.2 22", "DISPLAY": ":0"}):
|
||||
assert is_headless_environment() is True
|
||||
|
||||
with patch.dict(os.environ, {"SSH_CLIENT": "192.168.1.1 22 22", "DISPLAY": ":0"}):
|
||||
assert is_headless_environment() is True
|
||||
|
||||
def test_is_headless_wsl(self):
|
||||
"""Test that WSL environment is detected as headless."""
|
||||
# Skip this test on Windows since os.uname doesn't exist
|
||||
if not hasattr(os, "uname"):
|
||||
pytest.skip("os.uname not available on this platform")
|
||||
|
||||
with patch("sys.platform", "linux"):
|
||||
with patch("os.uname") as mock_uname:
|
||||
mock_uname.return_value = Mock(release="5.15.153.1-microsoft-standard-WSL2")
|
||||
with patch.dict(os.environ, {"DISPLAY": ":0"}):
|
||||
assert is_headless_environment() is True
|
||||
|
||||
def test_is_headless_docker(self):
|
||||
"""Test that Docker containers are detected as headless."""
|
||||
with patch("sys.platform", "linux"):
|
||||
# Test with CI environment variable
|
||||
with patch.dict(os.environ, {"CI": "true", "DISPLAY": ":0"}):
|
||||
assert is_headless_environment() is True
|
||||
|
||||
# Test with CONTAINER environment variable
|
||||
with patch.dict(os.environ, {"CONTAINER": "docker", "DISPLAY": ":0"}):
|
||||
assert is_headless_environment() is True
|
||||
|
||||
# Test with .dockerenv file
|
||||
with patch("os.path.exists") as mock_exists:
|
||||
mock_exists.return_value = True
|
||||
with patch.dict(os.environ, {"DISPLAY": ":0"}):
|
||||
assert is_headless_environment() is True
|
||||
|
||||
def test_is_not_headless_windows(self):
|
||||
"""Test that Windows is never detected as headless."""
|
||||
with patch("sys.platform", "win32"):
|
||||
# Even without DISPLAY, Windows should not be headless
|
||||
with patch.dict(os.environ, {}, clear=True):
|
||||
assert is_headless_environment() is False
|
||||
|
||||
|
||||
class TestShowFatalExceptionSafe:
|
||||
"""Test class for safe fatal exception display functionality."""
|
||||
|
||||
@patch("serena.util.exception.is_headless_environment", return_value=True)
|
||||
@patch("serena.util.exception.log")
|
||||
def test_show_fatal_exception_safe_headless(self, mock_log, mock_is_headless):
|
||||
"""Test that GUI is not attempted in headless environment."""
|
||||
test_exception = ValueError("Test error")
|
||||
|
||||
# The import should never happen in headless mode
|
||||
with patch("serena.gui_log_viewer.show_fatal_exception") as mock_show_gui:
|
||||
show_fatal_exception_safe(test_exception)
|
||||
mock_show_gui.assert_not_called()
|
||||
|
||||
# Verify debug log about skipping GUI
|
||||
mock_log.debug.assert_called_once_with("Skipping GUI error display in headless environment")
|
||||
|
||||
@patch("serena.util.exception.is_headless_environment", return_value=False)
|
||||
@patch("serena.util.exception.log")
|
||||
def test_show_fatal_exception_safe_with_gui(self, mock_log, mock_is_headless):
|
||||
"""Test that GUI is attempted when not in headless environment."""
|
||||
test_exception = ValueError("Test error")
|
||||
|
||||
# Mock the GUI function
|
||||
with patch("serena.gui_log_viewer.show_fatal_exception") as mock_show_gui:
|
||||
show_fatal_exception_safe(test_exception)
|
||||
mock_show_gui.assert_called_once_with(test_exception)
|
||||
|
||||
@patch("serena.util.exception.is_headless_environment", return_value=False)
|
||||
@patch("serena.util.exception.log")
|
||||
def test_show_fatal_exception_safe_gui_failure(self, mock_log, mock_is_headless):
|
||||
"""Test graceful handling when GUI display fails."""
|
||||
test_exception = ValueError("Test error")
|
||||
gui_error = ImportError("No module named 'tkinter'")
|
||||
|
||||
# Mock the GUI function to raise an exception
|
||||
with patch("serena.gui_log_viewer.show_fatal_exception", side_effect=gui_error):
|
||||
show_fatal_exception_safe(test_exception)
|
||||
|
||||
# Verify debug log about GUI failure
|
||||
mock_log.debug.assert_called_with(f"Failed to show GUI error dialog: {gui_error}")
|
||||
|
||||
def test_show_fatal_exception_safe_prints_to_stderr(self):
|
||||
"""Test that exceptions are always printed to stderr."""
|
||||
test_exception = ValueError("Test error message")
|
||||
|
||||
with patch("sys.stderr", new_callable=MagicMock) as mock_stderr:
|
||||
with patch("serena.util.exception.is_headless_environment", return_value=True):
|
||||
with patch("serena.util.exception.log"):
|
||||
show_fatal_exception_safe(test_exception)
|
||||
|
||||
# Verify print was called with the correct arguments
|
||||
mock_stderr.write.assert_any_call("Fatal exception: Test error message")
|
||||
Reference in new issue
Block a user