Add MCP Ping Support with Test Infrastructure Improvements#9
Open
abigagli wants to merge 6 commits into
Open
Conversation
- Implement ping handler in MCP server per specification
- Returns empty object {} as required by MCP spec
- No parameters needed, synchronous operation
- Fix critical IPC server bug preventing multiple requests
- Set accepted streams to blocking mode explicitly
- Prevents EAGAIN/EWOULDBLOCK errors on sequential requests
- Improves overall test infrastructure reliability
- Add comprehensive integration tests for ping
- Test basic ping response format
- Verify idempotency with multiple pings
- Test rapid-fire throughput
- Ensure server responsiveness after pings
Closes: MCP ping utility specification compliance
Spec: https://modelcontextprotocol.io/specification/2025-03-26/basic/utilities/ping
- Canonicalize workspace paths before passing to server - Ensures absolute paths are used consistently - Fixes file not found errors in wait_for_ready - Shorten Unix socket paths to avoid SUN_LEN limit - Use 'ra-mcp' instead of 'rust-analyzer-mcp-sockets' - Map project types to short socket names (e.g., 'diag.sock') - Fixes 'path must be shorter than SUN_LEN' error on macOS - SUN_LEN limit is typically 104 bytes, was exceeding with long paths These fixes enable all diagnostics integration tests to pass: - test_file_diagnostics - test_file_diagnostics_clean_file - test_diagnostics_severity_levels - test_workspace_diagnostics All 11 integration tests now pass successfully.
Add 6 new stress tests for ping functionality: 1. test_ping_rapid_fire - Tests sequential ping throughput (100-500 pings) - Measures requests per second performance 2. test_ping_concurrent - Tests concurrent ping handling (10-50 concurrent) - Verifies parallel request processing 3. test_ping_interleaved_with_tools - Alternates ping with actual tool calls (20-50 iterations) - Ensures ping doesn't interfere with LSP operations 4. test_ping_mixed_concurrent_workload - Concurrent mix of pings and tool calls (15-30 concurrent) - Tests realistic usage patterns with 40% pings 5. test_ping_burst_pattern - Multiple bursts of rapid pings (5-10 bursts of 20-50 pings) - Tests sustained high-frequency ping patterns 6. test_ping_connection_stability - Creates/destroys connections with pings (50-100 iterations) - Verifies no resource leaks or connection issues All tests include CI-specific tuning with lower iteration counts and validate that ping returns empty object per MCP spec. All 12 stress tests (6 ping + 6 original) pass successfully.
When set_workspace was called as the first tool invocation, ensure_client_started() would spawn rust-analyzer with the default workspace root (often '/') before set_workspace had a chance to update the path. This caused rust-analyzer to attempt indexing the entire filesystem, leading to inevitable timeouts on large projects. Skip the eager client start for set_workspace since it manages the client lifecycle itself (shutdown old client, update path, start new).
Add a Drop implementation for RustAnalyzerClient that synchronously kills the child rust-analyzer process via start_kill(). This prevents orphaned rust-analyzer processes when the MCP server exits unexpectedly. Also handle SIGTERM in addition to SIGINT so the graceful shutdown path (LSP shutdown/exit + process kill + wait) is reached when the parent process (e.g., Windsurf) terminates the MCP server.
Multiple concurrent tests calling IpcClient::get_or_create for the same project type would all fail to find an existing socket, then race to spawn their own server instance. Only one server wins the socket bind; the others die immediately, causing broken pipe errors in the tests. Fix by using an O_CREAT|O_EXCL lockfile as an atomic mutex around the check+spawn sequence. Waiting processes spin on the lock and re-check the socket once they acquire it, so only one server is ever started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the MCP
pingutility method as specified in the Model Context Protocol specification, along with fixes to the test infrastructure that improve reliability across the test suite.Implementation Details
Ping Handler
pingmethod handler insrc/mcp/server.rs{}per MCP specification requirementsTest Infrastructure Fixes
Socket Blocking Mode Fix:
Fixed a critical issue in the test IPC server where accepted connections inherited non-blocking mode from the listener socket, causing spurious
EAGAIN/EWOULDBLOCKerrors during request processing.Root Cause:
should_shutdownbetweenaccept()calls without blocking indefinitely)accept()returns connected sockets that inherit the listener's non-blocking flag (POSIX behavior)BufReader::read_line(), non-blocking mode causes reads to fail withWouldBlockif data isn't immediately available in the kernel bufferSolution:
Explicitly set accepted connections to blocking mode with
stream.set_nonblocking(false)immediately afteraccept(). This allows the listener to remain non-blocking for shutdown responsiveness while ensuring connected sockets reliably wait for complete request data.Additional Infrastructure Improvements:
test-support/src/ipc/client.rsto ensure consistent absolute path handlingIpcClient::send_requestfor better failure detectionTest Coverage
Integration Tests (
tests/integration/mcp_server_test.rs):Stress Tests (
tests/stress/concurrent_requests.rs):test_ping_rapid_fire: Sequential throughput (100-500 requests)test_ping_concurrent: Concurrent request handling (10-50 parallel)test_ping_interleaved_with_tools: Ping/tool call alternation (20-50 iterations)test_ping_mixed_concurrent_workload: Mixed concurrent workload with 40% ping ratio (15-30 concurrent)test_ping_burst_pattern: Sustained burst patterns (5-10 bursts of 20-50 requests)test_ping_connection_stability: Connection lifecycle testing (50-100 iterations)All tests include CI-specific tuning with reduced iteration counts for GitHub Actions environment.