From f76ed700fa159599623a4a44db49308c1c485ff8 Mon Sep 17 00:00:00 2001 From: Joshua Shanks Date: Sun, 6 Jul 2025 14:21:41 -0700 Subject: [PATCH] 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 --- .vscode/settings.json | 1 - src/serena/config/serena_config.py | 11 +- src/serena/util/exception.py | 46 +++++++- test/serena/config/__init__.py | 1 + test/serena/config/test_serena_config.py | 138 +++++++++++++++++++++++ test/serena/util/test_exception.py | 116 +++++++++++++++++++ 6 files changed, 307 insertions(+), 6 deletions(-) create mode 100644 test/serena/config/__init__.py create mode 100644 test/serena/config/test_serena_config.py create mode 100644 test/serena/util/test_exception.py diff --git a/.vscode/settings.json b/.vscode/settings.json index 6d3f3ba2..6c3ee2ca 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -12,4 +12,3 @@ "vibing" ], } - diff --git a/src/serena/config/serena_config.py b/src/serena/config/serena_config.py index 883c5931..d53716e3 100644 --- a/src/serena/config/serena_config.py +++ b/src/serena/config/serena_config.py @@ -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]) diff --git a/src/serena/util/exception.py b/src/serena/util/exception.py index af194314..8ab2f007 100644 --- a/src/serena/util/exception.py +++ b/src/serena/util/exception.py @@ -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}") diff --git a/test/serena/config/__init__.py b/test/serena/config/__init__.py new file mode 100644 index 00000000..6b95e0de --- /dev/null +++ b/test/serena/config/__init__.py @@ -0,0 +1 @@ +# Empty init file for test package diff --git a/test/serena/config/test_serena_config.py b/test/serena/config/test_serena_config.py new file mode 100644 index 00000000..14eb3cd9 --- /dev/null +++ b/test/serena/config/test_serena_config.py @@ -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) diff --git a/test/serena/util/test_exception.py b/test/serena/util/test_exception.py new file mode 100644 index 00000000..e13e57f7 --- /dev/null +++ b/test/serena/util/test_exception.py @@ -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")