From b197be488a17f3713507eea206445e7ce67b8c5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Thu, 21 Aug 2025 19:33:00 +0800 Subject: [PATCH 1/7] Fix Neovim process cleanup in integration tests Update integration tests to properly use NeovimIpcGuard for process lifecycle management. This ensures Neovim instances are correctly terminated after test completion, preventing resource leaks in the test suite. Changes: - Replace direct guard assignment with explicit NeovimIpcGuard::new() - Apply fix to organize_imports and lua_tools test functions - Maintain consistent variable naming for IPC paths --- src/server/integration_tests.rs | 52 ++++++++++++++++----------------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/src/server/integration_tests.rs b/src/server/integration_tests.rs index 81e9036..996e303 100644 --- a/src/server/integration_tests.rs +++ b/src/server/integration_tests.rs @@ -959,12 +959,13 @@ async fn test_lsp_organize_imports_non_existent_file() -> Result<(), Box Result<(), Box Result<(), Box Result<(), Box Date: Thu, 21 Aug 2025 20:52:04 +0800 Subject: [PATCH 2/7] Optimize integration tests with one-time binary compilation Replace per-test cargo run with pre-compiled binary approach: - Add get_compiled_binary() function with OnceLock for single compilation - Create create_mcp_service! macro for consistent binary usage - Reduce test execution time by eliminating redundant compilations - Maintain same test functionality with improved performance --- src/server/integration_tests.rs | 242 ++++++++++---------------------- 1 file changed, 74 insertions(+), 168 deletions(-) diff --git a/src/server/integration_tests.rs b/src/server/integration_tests.rs index 996e303..b095890 100644 --- a/src/server/integration_tests.rs +++ b/src/server/integration_tests.rs @@ -1,10 +1,12 @@ +use std::path::PathBuf; +use std::sync::OnceLock; use std::time::Duration; use rmcp::{ model::{CallToolRequestParam, ReadResourceRequestParam}, serde_json::{Map, Value}, service::ServiceExt, - transport::{ConfigureCommandExt, TokioChildProcess}, + transport::TokioChildProcess, }; use tokio::{process::Command, time}; use tracing::{error, info}; @@ -12,6 +14,60 @@ use tracing_test::traced_test; use crate::test_utils::*; +static BINARY_PATH: OnceLock = OnceLock::new(); + +/// Get the compiled binary path, compiling only once +fn get_compiled_binary() -> PathBuf { + BINARY_PATH + .get_or_init(|| { + info!("Compiling nvim-mcp binary (one-time compilation)..."); + + // Build the binary using cargo build + let output = std::process::Command::new("cargo") + .args(["build", "--bin", "nvim-mcp"]) + .output() + .expect("Failed to execute cargo build"); + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + panic!("Failed to compile nvim-mcp binary: {}", stderr); + } + + // Determine the binary path + let manifest_dir = + std::env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not set"); + let mut binary_path = PathBuf::from(manifest_dir); + binary_path.push("target"); + binary_path.push("debug"); + binary_path.push("nvim-mcp"); + + // On Windows, add .exe extension + #[cfg(windows)] + binary_path.set_extension("exe"); + + if !binary_path.exists() { + panic!("Binary not found at expected path: {:?}", binary_path); + } + + info!("Binary compiled successfully at: {:?}", binary_path); + binary_path + }) + .clone() +} + +/// Macro to create an MCP service using the pre-compiled binary +macro_rules! create_mcp_service { + () => {{ + let binary_path = get_compiled_binary(); + ().serve(TokioChildProcess::new(Command::new(binary_path))?) + .await + .map_err(|e| { + error!("Failed to connect to server: {}", e); + e + })? + }}; +} + // Helper function to extract connection_id from connect response fn extract_connection_id( result: &rmcp::model::CallToolResult, @@ -37,18 +93,8 @@ fn extract_connection_id( async fn test_mcp_server_connection() -> Result<(), Box> { info!("Starting MCP client to test nvim-mcp server"); - // Connect to the server running as a child process (exact copy from original) - let service = () - .serve(TokioChildProcess::new(Command::new("cargo").configure( - |cmd| { - cmd.args(["run", "--bin", "nvim-mcp"]); - }, - ))?) - .await - .map_err(|e| { - error!("Failed to connect to server: {}", e); - e - })?; + // Connect to the server using pre-compiled binary + let service = create_mcp_service!(); // Get server information let server_info = service.peer_info(); @@ -79,17 +125,7 @@ async fn test_mcp_server_connection() -> Result<(), Box> async fn test_list_tools() -> Result<(), Box> { info!("Starting MCP client to test nvim-mcp server"); - let service = () - .serve(TokioChildProcess::new(Command::new("cargo").configure( - |cmd| { - cmd.args(["run", "--bin", "nvim-mcp"]); - }, - ))?) - .await - .map_err(|e| { - error!("Failed to connect to server: {}", e); - e - })?; + let service = create_mcp_service!(); // List available tools let tools = service.list_tools(Default::default()).await?; @@ -121,17 +157,7 @@ async fn test_list_tools() -> Result<(), Box> { async fn test_connect_nvim_tcp_tool() -> Result<(), Box> { info!("Starting MCP client to test nvim-mcp server"); - let service = () - .serve(TokioChildProcess::new(Command::new("cargo").configure( - |cmd| { - cmd.args(["run", "--bin", "nvim-mcp"]); - }, - ))?) - .await - .map_err(|e| { - error!("Failed to connect to server: {}", e); - e - })?; + let service = create_mcp_service!(); // Start a test Neovim instance let ipc_path = generate_random_ipc_path(); @@ -193,17 +219,7 @@ async fn test_connect_nvim_tcp_tool() -> Result<(), Box> async fn test_disconnect_nvim_tcp_tool() -> Result<(), Box> { info!("Starting MCP client to test nvim-mcp server"); - let service = () - .serve(TokioChildProcess::new(Command::new("cargo").configure( - |cmd| { - cmd.args(["run", "--bin", "nvim-mcp"]); - }, - ))?) - .await - .map_err(|e| { - error!("Failed to connect to server: {}", e); - e - })?; + let service = create_mcp_service!(); // Test disconnect without valid connection (should fail) let mut invalid_disconnect_args = Map::new(); @@ -298,17 +314,7 @@ async fn test_disconnect_nvim_tcp_tool() -> Result<(), Box Result<(), Box> { info!("Starting MCP client to test nvim-mcp server"); - let service = () - .serve(TokioChildProcess::new(Command::new("cargo").configure( - |cmd| { - cmd.args(["run", "--bin", "nvim-mcp"]); - }, - ))?) - .await - .map_err(|e| { - error!("Failed to connect to server: {}", e); - e - })?; + let service = create_mcp_service!(); // Test list buffers without connection (should fail) let mut invalid_args = Map::new(); @@ -388,17 +394,7 @@ async fn test_list_buffers_tool() -> Result<(), Box> { async fn test_complete_workflow() -> Result<(), Box> { info!("Starting MCP client to test nvim-mcp server"); - let service = () - .serve(TokioChildProcess::new(Command::new("cargo").configure( - |cmd| { - cmd.args(["run", "--bin", "nvim-mcp"]); - }, - ))?) - .await - .map_err(|e| { - error!("Failed to connect to server: {}", e); - e - })?; + let service = create_mcp_service!(); // Start Neovim instance let ipc_path = generate_random_ipc_path(); @@ -507,17 +503,7 @@ async fn test_complete_workflow() -> Result<(), Box> { async fn test_error_handling() -> Result<(), Box> { info!("Starting MCP client to test nvim-mcp server"); - let service = () - .serve(TokioChildProcess::new(Command::new("cargo").configure( - |cmd| { - cmd.args(["run", "--bin", "nvim-mcp"]); - }, - ))?) - .await - .map_err(|e| { - error!("Failed to connect to server: {}", e); - e - })?; + let service = create_mcp_service!(); // Test connecting to invalid address let mut invalid_args = Map::new(); @@ -569,17 +555,7 @@ async fn test_error_handling() -> Result<(), Box> { async fn test_exec_lua_tool() -> Result<(), Box> { info!("Starting MCP client to test nvim-mcp server"); - let service = () - .serve(TokioChildProcess::new(Command::new("cargo").configure( - |cmd| { - cmd.args(["run", "--bin", "nvim-mcp"]); - }, - ))?) - .await - .map_err(|e| { - error!("Failed to connect to server: {}", e); - e - })?; + let service = create_mcp_service!(); // Test exec_lua without connection (should fail) let mut lua_args = Map::new(); @@ -729,17 +705,7 @@ async fn test_exec_lua_tool() -> Result<(), Box> { async fn test_lsp_clients_tool() -> Result<(), Box> { info!("Starting MCP client to test nvim-mcp server"); - let service = () - .serve(TokioChildProcess::new(Command::new("cargo").configure( - |cmd| { - cmd.args(["run", "--bin", "nvim-mcp"]); - }, - ))?) - .await - .map_err(|e| { - error!("Failed to connect to server: {}", e); - e - })?; + let service = create_mcp_service!(); // Test lsp_clients without connection (should fail) let mut invalid_args = Map::new(); @@ -811,17 +777,7 @@ async fn test_lsp_clients_tool() -> Result<(), Box> { async fn test_list_diagnostic_resources() -> Result<(), Box> { info!("Starting MCP client to test diagnostic resources"); - let service = () - .serve(TokioChildProcess::new(Command::new("cargo").configure( - |cmd| { - cmd.args(["run", "--bin", "nvim-mcp"]); - }, - ))?) - .await - .map_err(|e| { - error!("Failed to connect to server: {}", e); - e - })?; + let service = create_mcp_service!(); // Test list_resources let result = service.list_resources(None).await?; @@ -857,17 +813,7 @@ async fn test_list_diagnostic_resources() -> Result<(), Box Result<(), Box> { info!("Starting MCP client to test reading workspace diagnostics"); - let service = () - .serve(TokioChildProcess::new(Command::new("cargo").configure( - |cmd| { - cmd.args(["run", "--bin", "nvim-mcp"]); - }, - ))?) - .await - .map_err(|e| { - error!("Failed to connect to server: {}", e); - e - })?; + let service = create_mcp_service!(); // Start Neovim instance let ipc_path = generate_random_ipc_path(); @@ -945,17 +891,7 @@ async fn test_read_workspace_diagnostics() -> Result<(), Box Result<(), Box> { info!("Testing lsp_organize_imports with non-existent file"); - let service = () - .serve(TokioChildProcess::new(Command::new("cargo").configure( - |cmd| { - cmd.args(["run", "--bin", "nvim-mcp"]); - }, - ))?) - .await - .map_err(|e| { - error!("Failed to connect to server: {}", e); - e - })?; + let service = create_mcp_service!(); // Start a test Neovim instance let ipc_path = generate_random_ipc_path(); @@ -1023,17 +959,7 @@ async fn test_lsp_organize_imports_non_existent_file() -> Result<(), Box Result<(), Box> { info!("Testing lsp_organize_imports with LSP setup"); - let service = () - .serve(TokioChildProcess::new(Command::new("cargo").configure( - |cmd| { - cmd.args(["run", "--bin", "nvim-mcp"]); - }, - ))?) - .await - .map_err(|e| { - error!("Failed to connect to server: {}", e); - e - })?; + let service = create_mcp_service!(); // Start a test Neovim instance with LSP let ipc_path = generate_random_ipc_path(); @@ -1111,17 +1037,7 @@ async fn test_lsp_organize_imports_with_lsp() -> Result<(), Box Result<(), Box> { info!("Testing lsp_organize_imports in inspect mode (apply_edits=false)"); - let service = () - .serve(TokioChildProcess::new(Command::new("cargo").configure( - |cmd| { - cmd.args(["run", "--bin", "nvim-mcp"]); - }, - ))?) - .await - .map_err(|e| { - error!("Failed to connect to server: {}", e); - e - })?; + let service = create_mcp_service!(); // Start a test Neovim instance with LSP let ipc_path = generate_random_ipc_path(); @@ -1200,18 +1116,8 @@ async fn test_lsp_organize_imports_inspect_mode() -> Result<(), Box Result<(), Box> { info!("Testing end-to-end Lua tools workflow"); - // Start MCP server with child process - let service = () - .serve(TokioChildProcess::new(Command::new("cargo").configure( - |cmd| { - cmd.args(["run", "--bin", "nvim-mcp"]); - }, - ))?) - .await - .map_err(|e| { - error!("Failed to connect to server: {}", e); - e - })?; + // Connect to pre-compiled MCP server + let service = create_mcp_service!(); info!("Connected to server"); From 0cd895d3baf497182c298918a830ec1dd01e254f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Thu, 21 Aug 2025 21:40:00 +0800 Subject: [PATCH 3/7] Optimize integration test timing for better performance - Reduce LSP initialization wait times from 15-20s to 2-3s - Decrease save operation wait from 1000ms to 100ms - Reduce connection polling interval from 100ms to 50ms - Optimize Lua tools test wait from 2000ms to 500ms - Maintain test reliability while significantly improving speed --- src/neovim/integration_tests.rs | 30 +++++++++++++++--------------- src/server/integration_tests.rs | 6 +++--- src/test_utils.rs | 8 ++++---- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/neovim/integration_tests.rs b/src/neovim/integration_tests.rs index 57cea07..950751a 100644 --- a/src/neovim/integration_tests.rs +++ b/src/neovim/integration_tests.rs @@ -196,7 +196,7 @@ async fn test_get_vim_diagnostics() { "Failed to setup diagnostics autocmd: {result:?}" ); - sleep(Duration::from_secs(20)).await; // Allow time for LSP to initialize + sleep(Duration::from_secs(3)).await; // Allow time for LSP to initialize let result = client.get_buffer_diagnostics(0).await; assert!(result.is_ok(), "Failed to get diagnostics: {result:?}"); @@ -232,7 +232,7 @@ async fn test_code_action() { "Failed to setup diagnostics autocmd: {result:?}" ); - sleep(Duration::from_secs(20)).await; // Allow time for LSP to initialize + sleep(Duration::from_secs(3)).await; // Allow time for LSP to initialize let result = client.get_buffer_diagnostics(0).await; assert!(result.is_ok(), "Failed to get diagnostics: {result:?}"); @@ -296,7 +296,7 @@ async fn test_lsp_resolve_code_action() { "Failed to setup diagnostics autocmd: {result:?}" ); - sleep(Duration::from_secs(20)).await; // Allow time for LSP to initialize + sleep(Duration::from_secs(3)).await; // Allow time for LSP to initialize // Position cursor inside fmt.Println call (line 6, character 6) let result = client @@ -415,7 +415,7 @@ async fn test_lsp_apply_workspace_edit() { "Failed to setup diagnostics autocmd: {result:?}" ); - sleep(Duration::from_secs(20)).await; // Allow time for LSP to initialize + sleep(Duration::from_secs(3)).await; // Allow time for LSP to initialize // Get buffer diagnostics to find modernization opportunities let result = client.get_buffer_diagnostics(0).await; @@ -473,7 +473,7 @@ async fn test_lsp_apply_workspace_edit() { assert!(result.is_ok(), "Failed to save buffer: {result:?}"); // Give some time for the edit and save to be applied - sleep(Duration::from_millis(1000)).await; + sleep(Duration::from_millis(100)).await; // Read the modified content to verify the change let modified_content = @@ -551,7 +551,7 @@ func main() { "Failed to setup diagnostics autocmd: {result:?}" ); - sleep(Duration::from_secs(15)).await; // Allow time for LSP to initialize + sleep(Duration::from_secs(2)).await; // Allow time for LSP to initialize // Get LSP clients let lsp_clients = client.lsp_get_clients().await.unwrap(); @@ -671,7 +671,7 @@ pub fn main() !void { "Failed to setup diagnostics autocmd: {result:?}" ); - sleep(Duration::from_secs(15)).await; // Allow time for LSP to initialize + sleep(Duration::from_secs(2)).await; // Allow time for LSP to initialize // Get LSP clients let lsp_clients = client.lsp_get_clients().await.unwrap(); @@ -789,7 +789,7 @@ func main() { "Failed to setup diagnostics autocmd: {result:?}" ); - sleep(Duration::from_secs(15)).await; // Allow time for LSP to initialize + sleep(Duration::from_secs(2)).await; // Allow time for LSP to initialize // Get LSP clients let lsp_clients = client.lsp_get_clients().await.unwrap(); @@ -913,7 +913,7 @@ func main() { "Failed to setup diagnostics autocmd: {result:?}" ); - sleep(Duration::from_secs(15)).await; // Allow time for LSP to initialize + sleep(Duration::from_secs(2)).await; // Allow time for LSP to initialize // Get LSP clients let lsp_clients = client.lsp_get_clients().await.unwrap(); @@ -1015,7 +1015,7 @@ async fn test_lsp_rename_with_prepare() { "Failed to setup diagnostics autocmd: {result:?}" ); - sleep(Duration::from_secs(20)).await; // Allow time for LSP to initialize + sleep(Duration::from_secs(3)).await; // Allow time for LSP to initialize // Get LSP clients let lsp_clients = client.lsp_get_clients().await.unwrap(); @@ -1062,7 +1062,7 @@ async fn test_lsp_rename_with_prepare() { assert!(result.is_ok(), "Failed to save buffer: {result:?}"); // Give some time for the save operation to complete - sleep(Duration::from_millis(1000)).await; + sleep(Duration::from_millis(100)).await; // Read the file content to verify the rename was applied let updated_content = @@ -1118,7 +1118,7 @@ async fn test_lsp_rename_without_prepare() { "Failed to setup diagnostics autocmd: {result:?}" ); - sleep(Duration::from_secs(20)).await; // Allow time for LSP to initialize + sleep(Duration::from_secs(3)).await; // Allow time for LSP to initialize // Get LSP clients let lsp_clients = client.lsp_get_clients().await.unwrap(); @@ -1165,7 +1165,7 @@ async fn test_lsp_rename_without_prepare() { assert!(result.is_ok(), "Failed to save buffer: {result:?}"); // Give some time for the save operation to complete - sleep(Duration::from_millis(1000)).await; + sleep(Duration::from_millis(100)).await; // Read the file content to verify the rename was applied let updated_content = @@ -1251,7 +1251,7 @@ main(); "Failed to setup diagnostics autocmd: {result:?}" ); - sleep(Duration::from_secs(20)).await; // Allow time for LSP to initialize + sleep(Duration::from_secs(3)).await; // Allow time for LSP to initialize (temp_dir, guard, client) } @@ -1348,7 +1348,7 @@ main(); "Failed to setup diagnostics autocmd: {result:?}" ); - sleep(Duration::from_secs(20)).await; // Allow time for LSP to initialize + sleep(Duration::from_secs(3)).await; // Allow time for LSP to initialize use crate::neovim::FormattingOptions; let tab_options = FormattingOptions { diff --git a/src/server/integration_tests.rs b/src/server/integration_tests.rs index b095890..5358769 100644 --- a/src/server/integration_tests.rs +++ b/src/server/integration_tests.rs @@ -971,7 +971,7 @@ async fn test_lsp_organize_imports_with_lsp() -> Result<(), Box Result<(), Box Result<(), Box Date: Thu, 21 Aug 2025 23:41:27 +0800 Subject: [PATCH 4/7] Refactor autocmd setup and add notification tracking - Replace diagnostics_autocmd.lua with setup_autocmd.lua for enhanced autocmd management - Rename setup_diagnostics_changed_autocmd to setup_autocmd for cleaner API - Add NotificationTracker for async notification handling with timeout support - Add LspAttach and LspDetach autocmd handlers with detailed client information - Update all integration tests to use new method name - Enhance notification system with memory management and proper cleanup --- src/neovim/client.rs | 250 +++++++++++++++++++++++-- src/neovim/integration_tests.rs | 24 +-- src/neovim/lua/diagnostics_autocmd.lua | 17 -- src/neovim/lua/setup_autocmd.lua | 49 +++++ src/server/core.rs | 2 +- src/server/tools.rs | 4 +- 6 files changed, 303 insertions(+), 43 deletions(-) delete mode 100644 src/neovim/lua/diagnostics_autocmd.lua create mode 100644 src/neovim/lua/setup_autocmd.lua diff --git a/src/neovim/client.rs b/src/neovim/client.rs index 6e17d87..b76bac6 100644 --- a/src/neovim/client.rs +++ b/src/neovim/client.rs @@ -5,13 +5,19 @@ use std::fmt::{self, Display}; use std::marker::PhantomData; use std::path::{Path, PathBuf}; use std::str::FromStr; +use std::sync::Arc; use async_trait::async_trait; use nvim_rs::{Handler, Neovim, create::tokio as create}; use rmpv::Value; use serde::de::{self, MapAccess, Visitor}; use serde::{Deserialize, Deserializer}; -use tokio::{io::AsyncWrite, net::TcpStream}; +use tokio::{ + io::AsyncWrite, + net::TcpStream, + sync::Mutex, + time::{Duration, timeout}, +}; use tracing::{debug, info, instrument}; use super::{connection::NeovimConnection, error::NeovimError}; @@ -32,7 +38,14 @@ pub trait NeovimClientTrait: Sync { async fn execute_lua(&self, code: &str) -> Result; /// Set up diagnostics changed autocmd - async fn setup_diagnostics_changed_autocmd(&self) -> Result<(), NeovimError>; + async fn setup_autocmd(&self) -> Result<(), NeovimError>; + + /// Wait for a specific notification with timeout + async fn wait_for_notification( + &self, + notification_name: &str, + timeout_ms: u64, + ) -> Result; /// Get diagnostics for a specific buffer async fn get_buffer_diagnostics(&self, buffer_id: u64) -> Result, NeovimError>; @@ -178,22 +191,125 @@ pub trait NeovimClientTrait: Sync { ) -> Result<(), NeovimError>; } +/// Notification tracking structure +#[derive(Debug, Clone)] +pub struct Notification { + pub name: String, + pub args: Vec, + pub timestamp: std::time::SystemTime, +} + +/// Shared state for notification tracking +#[derive(Clone)] +pub struct NotificationTracker { + notifications: Arc>>, + notify_wakers: Arc>>>>, +} + +impl NotificationTracker { + pub fn new() -> Self { + Self { + notifications: Arc::new(Mutex::new(Vec::new())), + notify_wakers: Arc::new(Mutex::new(HashMap::new())), + } + } + + /// Record a notification + pub async fn record_notification(&self, name: String, args: Vec) { + let notification = Notification { + name: name.clone(), + args, + timestamp: std::time::SystemTime::now(), + }; + + // Notify any waiting tasks for this specific notification name first + let mut wakers = self.notify_wakers.lock().await; + if let Some(waiters) = wakers.get_mut(&name) { + while let Some(waker) = waiters.pop() { + let _ = waker.send(notification.clone()); + } + } + + // Only store the notification if there are no more waiters for this type + // This prevents memory leaks from accumulating notifications + if wakers.get(&name).is_none_or(|waiters| waiters.is_empty()) { + let mut notifications = self.notifications.lock().await; + notifications.push(notification); + } + } + + /// Wait for a specific notification with timeout + pub async fn wait_for_notification( + &self, + notification_name: &str, + timeout_duration: Duration, + ) -> Result { + // First check if the notification already exists + { + let notifications = self.notifications.lock().await; + if let Some(notification) = notifications + .iter() + .rev() + .find(|n| n.name == notification_name) + { + return Ok(notification.clone()); + } + } + + // Create a oneshot channel to wait for the notification + let (tx, rx) = tokio::sync::oneshot::channel(); + + // Register our interest in this notification + let mut wakers = self.notify_wakers.lock().await; + wakers + .entry(notification_name.to_string()) + .or_insert_with(Vec::new) + .push(tx); + + // Wait for the notification with timeout + drop(wakers); // Release the lock before awaiting + + match timeout(timeout_duration, rx).await { + Ok(Ok(notification)) => Ok(notification), + Ok(Err(_)) => Err(NeovimError::Api( + "Notification channel closed unexpectedly".to_string(), + )), + Err(_) => Err(NeovimError::Api(format!( + "Timeout waiting for notification: {}", + notification_name + ))), + } + } + + /// Clear all recorded notifications + pub async fn clear_notifications(&self) { + self.notifications.lock().await.clear(); + } +} + pub struct NeovimHandler { _marker: std::marker::PhantomData, + notification_tracker: NotificationTracker, } impl NeovimHandler { pub fn new() -> Self { NeovimHandler { _marker: std::marker::PhantomData, + notification_tracker: NotificationTracker::new(), } } + + pub fn notification_tracker(&self) -> NotificationTracker { + self.notification_tracker.clone() + } } impl Clone for NeovimHandler { fn clone(&self) -> Self { NeovimHandler { _marker: std::marker::PhantomData, + notification_tracker: self.notification_tracker.clone(), } } } @@ -207,6 +323,9 @@ where async fn handle_notify(&self, name: String, args: Vec, _neovim: Neovim) { info!("handling notification: {name:?}, {args:?}"); + self.notification_tracker + .record_notification(name, args) + .await; } async fn handle_request( @@ -1040,6 +1159,7 @@ where T: AsyncWrite + Send + 'static, { connection: Option>, + notification_tracker: Option, } #[cfg(unix)] @@ -1103,6 +1223,7 @@ impl NeovimClient { debug!("Attempting to connect to Neovim at {}", path); let handler = NeovimHandler::new(); + let notification_tracker = handler.notification_tracker(); match create::new_path(path, handler).await { Ok((nvim, io_handler)) => { let connection = NeovimConnection::new( @@ -1115,6 +1236,7 @@ impl NeovimClient { path.to_string(), ); self.connection = Some(connection); + self.notification_tracker = Some(notification_tracker); debug!("Successfully connected to Neovim at {}", path); Ok(()) } @@ -1138,6 +1260,7 @@ impl NeovimClient { debug!("Attempting to connect to Neovim at {}", address); let handler = NeovimHandler::new(); + let notification_tracker = handler.notification_tracker(); match create::new_tcp(address, handler).await { Ok((nvim, io_handler)) => { let connection = NeovimConnection::new( @@ -1150,6 +1273,7 @@ impl NeovimClient { address.to_string(), ); self.connection = Some(connection); + self.notification_tracker = Some(notification_tracker); debug!("Successfully connected to Neovim at {}", address); Ok(()) } @@ -1166,7 +1290,10 @@ where T: AsyncWrite + Send + 'static, { pub fn new() -> Self { - Self { connection: None } + Self { + connection: None, + notification_tracker: None, + } } #[instrument(skip(self))] @@ -1309,6 +1436,12 @@ where if let Some(connection) = self.connection.take() { let target = connection.target().to_string(); connection.io_handler.abort(); + + // Clear notification tracker to free memory + if let Some(tracker) = self.notification_tracker.take() { + tracker.clear_notifications().await; + } + debug!("Successfully disconnected from Neovim at {}", target); Ok(target) } else { @@ -1371,8 +1504,8 @@ where } #[instrument(skip(self))] - async fn setup_diagnostics_changed_autocmd(&self) -> Result<(), NeovimError> { - debug!("Setting up diagnostics changed autocmd"); + async fn setup_autocmd(&self) -> Result<(), NeovimError> { + debug!("Setting up autocmd"); let conn = self.connection.as_ref().ok_or_else(|| { NeovimError::Connection("Not connected to any Neovim instance".to_string()) @@ -1380,18 +1513,16 @@ where match conn .nvim - .exec_lua(include_str!("lua/diagnostics_autocmd.lua"), vec![]) + .exec_lua(include_str!("lua/setup_autocmd.lua"), vec![]) .await { Ok(_) => { - debug!("Autocmd for diagnostics changed set up successfully"); + debug!("autocmd set up successfully"); Ok(()) } Err(e) => { - debug!("Failed to set up diagnostics changed autocmd: {}", e); - Err(NeovimError::Api(format!( - "Failed to set up diagnostics changed autocmd: {e}" - ))) + debug!("Failed to set up autocmd: {}", e); + Err(NeovimError::Api(format!("Failed to set up autocmd: {e}"))) } } } @@ -2393,6 +2524,26 @@ where } } } + + #[instrument(skip(self))] + async fn wait_for_notification( + &self, + notification_name: &str, + timeout_ms: u64, + ) -> Result { + debug!( + "Waiting for notification: {} with timeout: {}ms", + notification_name, timeout_ms + ); + + let tracker = self.notification_tracker.as_ref().ok_or_else(|| { + NeovimError::Connection("Not connected to any Neovim instance".to_string()) + })?; + + tracker + .wait_for_notification(notification_name, Duration::from_millis(timeout_ms)) + .await + } } #[cfg(test)] @@ -2902,4 +3053,81 @@ mod tests { let deserialized = deserialized.workspace_edit; assert!(deserialized.changes.is_some()); } + + #[tokio::test] + async fn test_notification_tracker_basic() { + let tracker = NotificationTracker::new(); + + // Test recording a notification + tracker + .record_notification( + "test_notification".to_string(), + vec![Value::from("test_arg")], + ) + .await; + + // Test waiting for the notification + let result = tracker + .wait_for_notification("test_notification", Duration::from_millis(100)) + .await; + + assert!(result.is_ok()); + let notification = result.unwrap(); + assert_eq!(notification.name, "test_notification"); + assert_eq!(notification.args.len(), 1); + assert_eq!(notification.args[0].as_str().unwrap(), "test_arg"); + } + + #[tokio::test] + async fn test_notification_tracker_timeout() { + let tracker = NotificationTracker::new(); + + // Test waiting for a notification that never comes (should timeout) + let result = tracker + .wait_for_notification("nonexistent_notification", Duration::from_millis(50)) + .await; + + assert!(result.is_err()); + let error = result.unwrap_err(); + assert!(matches!(error, NeovimError::Api(_))); + assert!( + error + .to_string() + .contains("Timeout waiting for notification") + ); + } + + #[tokio::test] + async fn test_notification_tracker_wait_then_send() { + let tracker = NotificationTracker::new(); + + // Spawn a task that will wait for a notification + let wait_handle = tokio::spawn({ + let tracker = tracker.clone(); + async move { + tracker + .wait_for_notification("test_async_notification", Duration::from_millis(500)) + .await + } + }); + + // Give the waiting task a moment to start waiting + tokio::time::sleep(Duration::from_millis(10)).await; + + // Now send the notification + tracker + .record_notification( + "test_async_notification".to_string(), + vec![Value::from("async_test_arg")], + ) + .await; + + // The waiting task should now receive the notification + let result = wait_handle.await.unwrap(); + assert!(result.is_ok()); + let notification = result.unwrap(); + assert_eq!(notification.name, "test_async_notification"); + assert_eq!(notification.args.len(), 1); + assert_eq!(notification.args[0].as_str().unwrap(), "async_test_arg"); + } } diff --git a/src/neovim/integration_tests.rs b/src/neovim/integration_tests.rs index 950751a..f572361 100644 --- a/src/neovim/integration_tests.rs +++ b/src/neovim/integration_tests.rs @@ -190,7 +190,7 @@ async fn test_get_vim_diagnostics() { assert!(result.is_ok(), "Failed to connect to instance"); // Set up diagnostics and get diagnostics for buffer 0 - let result = client.setup_diagnostics_changed_autocmd().await; + let result = client.setup_autocmd().await; assert!( result.is_ok(), "Failed to setup diagnostics autocmd: {result:?}" @@ -226,7 +226,7 @@ async fn test_code_action() { assert!(result.is_ok(), "Failed to connect to instance"); // Set up diagnostics and wait for LSP - let result = client.setup_diagnostics_changed_autocmd().await; + let result = client.setup_autocmd().await; assert!( result.is_ok(), "Failed to setup diagnostics autocmd: {result:?}" @@ -290,7 +290,7 @@ async fn test_lsp_resolve_code_action() { assert!(result.is_ok(), "Failed to connect to instance"); // Set up diagnostics and wait for LSP - let result = client.setup_diagnostics_changed_autocmd().await; + let result = client.setup_autocmd().await; assert!( result.is_ok(), "Failed to setup diagnostics autocmd: {result:?}" @@ -409,7 +409,7 @@ async fn test_lsp_apply_workspace_edit() { assert!(result.is_ok(), "Failed to connect to instance"); // Set up diagnostics and wait for LSP - let result = client.setup_diagnostics_changed_autocmd().await; + let result = client.setup_autocmd().await; assert!( result.is_ok(), "Failed to setup diagnostics autocmd: {result:?}" @@ -545,7 +545,7 @@ func main() { assert!(result.is_ok(), "Failed to connect to instance"); // Set up diagnostics and wait for LSP - let result = client.setup_diagnostics_changed_autocmd().await; + let result = client.setup_autocmd().await; assert!( result.is_ok(), "Failed to setup diagnostics autocmd: {result:?}" @@ -665,7 +665,7 @@ pub fn main() !void { assert!(result.is_ok(), "Failed to connect to instance"); // Set up diagnostics and wait for LSP - let result = client.setup_diagnostics_changed_autocmd().await; + let result = client.setup_autocmd().await; assert!( result.is_ok(), "Failed to setup diagnostics autocmd: {result:?}" @@ -783,7 +783,7 @@ func main() { assert!(result.is_ok(), "Failed to connect to instance"); // Set up diagnostics and wait for LSP - let result = client.setup_diagnostics_changed_autocmd().await; + let result = client.setup_autocmd().await; assert!( result.is_ok(), "Failed to setup diagnostics autocmd: {result:?}" @@ -907,7 +907,7 @@ func main() { assert!(result.is_ok(), "Failed to connect to instance"); // Set up diagnostics and wait for LSP - let result = client.setup_diagnostics_changed_autocmd().await; + let result = client.setup_autocmd().await; assert!( result.is_ok(), "Failed to setup diagnostics autocmd: {result:?}" @@ -1009,7 +1009,7 @@ async fn test_lsp_rename_with_prepare() { assert!(result.is_ok(), "Failed to connect to instance"); // Set up diagnostics and wait for LSP - let result = client.setup_diagnostics_changed_autocmd().await; + let result = client.setup_autocmd().await; assert!( result.is_ok(), "Failed to setup diagnostics autocmd: {result:?}" @@ -1112,7 +1112,7 @@ async fn test_lsp_rename_without_prepare() { assert!(result.is_ok(), "Failed to connect to instance"); // Set up diagnostics and wait for LSP - let result = client.setup_diagnostics_changed_autocmd().await; + let result = client.setup_autocmd().await; assert!( result.is_ok(), "Failed to setup diagnostics autocmd: {result:?}" @@ -1245,7 +1245,7 @@ main(); assert!(result.is_ok(), "Failed to connect to instance"); // Set up diagnostics and wait for LSP - let result = client.setup_diagnostics_changed_autocmd().await; + let result = client.setup_autocmd().await; assert!( result.is_ok(), "Failed to setup diagnostics autocmd: {result:?}" @@ -1342,7 +1342,7 @@ main(); assert!(result.is_ok(), "Failed to connect to instance"); // Set up diagnostics and wait for LSP - let result = client.setup_diagnostics_changed_autocmd().await; + let result = client.setup_autocmd().await; assert!( result.is_ok(), "Failed to setup diagnostics autocmd: {result:?}" diff --git a/src/neovim/lua/diagnostics_autocmd.lua b/src/neovim/lua/diagnostics_autocmd.lua deleted file mode 100644 index a6c2345..0000000 --- a/src/neovim/lua/diagnostics_autocmd.lua +++ /dev/null @@ -1,17 +0,0 @@ -local group = vim.api.nvim_create_augroup("NVIM_MCP_DiagnosticsChanged", { clear = true }) -vim.api.nvim_create_autocmd("DiagnosticChanged", { - group = group, - callback = function(args) - vim.rpcnotify(0, "NVIM_MCP_DiagnosticsChanged", { - buf = args.buf, - diagnostics = args.data.diagnostics, - }) - end, -}) -vim.api.nvim_create_autocmd("LspAttach", { - group = group, - callback = function(args) - vim.rpcnotify(0, "NVIM_MCP_LspAttach", args.data.diagnostics) - end, -}) -vim.rpcnotify(0, "NVIM_MCP", "setup diagnostics changed autocmd") diff --git a/src/neovim/lua/setup_autocmd.lua b/src/neovim/lua/setup_autocmd.lua new file mode 100644 index 0000000..39d20b1 --- /dev/null +++ b/src/neovim/lua/setup_autocmd.lua @@ -0,0 +1,49 @@ +local group = vim.api.nvim_create_augroup("NVIM_MCP_DiagnosticsChanged", { clear = true }) + +vim.api.nvim_create_autocmd("DiagnosticChanged", { + group = group, + callback = function(args) + vim.rpcnotify(0, "NVIM_MCP_DiagnosticsChanged", { + buf = args.buf, + diagnostics = args.data.diagnostics, + }) + end, +}) + +vim.api.nvim_create_autocmd("LspAttach", { + group = group, + callback = function(args) + local client_id = args.data.client_id + local client = vim.lsp.get_client_by_id(client_id) + + if client then + vim.rpcnotify(0, "NVIM_MCP_LspAttach", { + client_name = client.name, + client_id = client_id, + buffer_id = args.buf, + server_capabilities = client.server_capabilities, + initialized = client.initialized, + attach_time = vim.uv.now(), + }) + end + end, +}) + +vim.api.nvim_create_autocmd("LspDetach", { + group = group, + callback = function(args) + local client_id = args.data.client_id + local client = vim.lsp.get_client_by_id(client_id) + + if client then + vim.rpcnotify(0, "NVIM_MCP_LspDetach", { + client_name = client.name, + client_id = client_id, + buffer_id = args.buf, + detach_time = vim.uv.now(), + }) + end + end, +}) + +vim.rpcnotify(0, "NVIM_MCP", "setup diagnostics changed autocmd") diff --git a/src/server/core.rs b/src/server/core.rs index 6c64714..8f8526c 100644 --- a/src/server/core.rs +++ b/src/server/core.rs @@ -240,7 +240,7 @@ pub async fn auto_connect_single_target( // Import NeovimClient here to avoid circular imports let mut client = crate::neovim::NeovimClient::new(); client.connect_path(target).await?; - client.setup_diagnostics_changed_autocmd().await?; + client.setup_autocmd().await?; server .nvim_clients diff --git a/src/server/tools.rs b/src/server/tools.rs index a2b4e8a..aae4ca2 100644 --- a/src/server/tools.rs +++ b/src/server/tools.rs @@ -358,7 +358,7 @@ impl NeovimMcpServer { let mut client = NeovimClient::new(); client.connect_path(&path).await?; - client.setup_diagnostics_changed_autocmd().await?; + client.setup_autocmd().await?; // Discover and register Lua tools for this connection if let Err(e) = @@ -410,7 +410,7 @@ impl NeovimMcpServer { let mut client = NeovimClient::new(); client.connect_tcp(&address).await?; - client.setup_diagnostics_changed_autocmd().await?; + client.setup_autocmd().await?; // Discover and register Lua tools for this connection if let Err(e) = From 085e5ff8e89b4f199d7a98d9ea3400653b118096 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Fri, 22 Aug 2025 19:20:00 +0800 Subject: [PATCH 5/7] Improve test reliability with robust LSP synchronization Add wait_for_lsp_ready and wait_for_diagnostics methods to properly synchronize tests with LSP initialization. Implement notification cleanup to prevent memory leaks and ensure test stability. Replace arbitrary sleep calls with deterministic LSP readiness checks. Key improvements: - Add wait_for_lsp_ready() and wait_for_diagnostics() methods to NeovimClientTrait - Implement notification expiry and cleanup mechanisms in NotificationTracker - Add helper functions for common LSP setup patterns in integration tests - Replace sleep-based timing with event-driven synchronization - Improve test reliability by waiting for actual LSP attachment events --- src/neovim/client.rs | 287 +++++++++++++++++++++++++++++++- src/neovim/integration_tests.rs | 187 +++++++++------------ src/server/integration_tests.rs | 52 +++++- src/server/tools.rs | 40 +++++ 4 files changed, 446 insertions(+), 120 deletions(-) diff --git a/src/neovim/client.rs b/src/neovim/client.rs index b76bac6..1981f10 100644 --- a/src/neovim/client.rs +++ b/src/neovim/client.rs @@ -47,6 +47,20 @@ pub trait NeovimClientTrait: Sync { timeout_ms: u64, ) -> Result; + /// Wait for LSP client to be ready and attached + async fn wait_for_lsp_ready( + &self, + client_name: Option<&str>, + timeout_ms: u64, + ) -> Result<(), NeovimError>; + + /// Wait for diagnostics to be available for a specific buffer or workspace + async fn wait_for_diagnostics( + &self, + buffer_id: Option, + timeout_ms: u64, + ) -> Result, NeovimError>; + /// Get diagnostics for a specific buffer async fn get_buffer_diagnostics(&self, buffer_id: u64) -> Result, NeovimError>; @@ -206,6 +220,10 @@ pub struct NotificationTracker { notify_wakers: Arc>>>>, } +/// Configuration for notification cleanup +const MAX_STORED_NOTIFICATIONS: usize = 100; +const NOTIFICATION_EXPIRY_SECONDS: u64 = 30; + impl NotificationTracker { pub fn new() -> Self { Self { @@ -214,6 +232,25 @@ impl NotificationTracker { } } + /// Clean up expired and excess notifications + async fn cleanup_notifications(&self) { + let mut notifications = self.notifications.lock().await; + + // Remove expired notifications + let now = std::time::SystemTime::now(); + notifications.retain(|n| { + now.duration_since(n.timestamp) + .map(|d| d.as_secs() < NOTIFICATION_EXPIRY_SECONDS) + .unwrap_or(false) + }); + + // If still too many notifications, keep only the most recent ones + if notifications.len() > MAX_STORED_NOTIFICATIONS { + let excess = notifications.len() - MAX_STORED_NOTIFICATIONS; + notifications.drain(0..excess); + } + } + /// Record a notification pub async fn record_notification(&self, name: String, args: Vec) { let notification = Notification { @@ -230,11 +267,21 @@ impl NotificationTracker { } } - // Only store the notification if there are no more waiters for this type - // This prevents memory leaks from accumulating notifications - if wakers.get(&name).is_none_or(|waiters| waiters.is_empty()) { + // Clean up wakers with no waiters + wakers.retain(|_, waiters| !waiters.is_empty()); + drop(wakers); // Release lock early + + // Always store recent notifications for potential future requests + // but clean up old/excess ones to prevent memory leaks + { let mut notifications = self.notifications.lock().await; notifications.push(notification); + + // Trigger cleanup if we're approaching the limit + if notifications.len() > MAX_STORED_NOTIFICATIONS * 3 / 4 { + drop(notifications); // Release lock before calling cleanup + self.cleanup_notifications().await; + } } } @@ -244,13 +291,21 @@ impl NotificationTracker { notification_name: &str, timeout_duration: Duration, ) -> Result { - // First check if the notification already exists + // First check if a recent (non-expired) notification already exists { let notifications = self.notifications.lock().await; + let now = std::time::SystemTime::now(); + if let Some(notification) = notifications .iter() - .rev() - .find(|n| n.name == notification_name) + .rev() // Check most recent first + .find(|n| { + n.name == notification_name + && now + .duration_since(n.timestamp) + .map(|d| d.as_secs() < NOTIFICATION_EXPIRY_SECONDS) + .unwrap_or(false) + }) { return Ok(notification.clone()); } @@ -283,7 +338,20 @@ impl NotificationTracker { /// Clear all recorded notifications pub async fn clear_notifications(&self) { - self.notifications.lock().await.clear(); + let mut notifications = self.notifications.lock().await; + notifications.clear(); + } + + /// Manually trigger cleanup of expired notifications + pub async fn cleanup_expired_notifications(&self) { + self.cleanup_notifications().await; + } + + /// Get current notification statistics (for debugging/monitoring) + pub async fn get_stats(&self) -> (usize, usize) { + let notifications = self.notifications.lock().await; + let wakers = self.notify_wakers.lock().await; + (notifications.len(), wakers.len()) } } @@ -2544,6 +2612,101 @@ where .wait_for_notification(notification_name, Duration::from_millis(timeout_ms)) .await } + + #[instrument(skip(self))] + async fn wait_for_lsp_ready( + &self, + client_name: Option<&str>, + timeout_ms: u64, + ) -> Result<(), NeovimError> { + debug!( + "Waiting for LSP client readiness: {:?} with timeout: {}ms", + client_name, timeout_ms + ); + + // Wait for NVIM_MCP_LspAttach notification + let notification = self + .wait_for_notification("NVIM_MCP_LspAttach", timeout_ms) + .await?; + + // If specific client name is requested, verify it matches + if let Some(expected_client_name) = client_name { + // Parse the notification args to check client name + if let Some(attach_data) = notification.args.first() { + // Extract client_name from the nvim_rs::Value + if let Value::Map(map) = attach_data { + // Find the client_name key-value pair in the map + let client_name_key = Value::String("client_name".into()); + let client_name_value = map + .iter() + .find(|(k, _)| k == &client_name_key) + .map(|(_, v)| v); + + if let Some(Value::String(actual_client_name)) = client_name_value { + let actual_str = actual_client_name.as_str().unwrap_or(""); + if actual_str != expected_client_name { + return Err(NeovimError::Api(format!( + "LSP client '{}' attached but expected '{}'", + actual_str, expected_client_name + ))); + } + } else { + return Err(NeovimError::Api( + "LSP attach notification missing or invalid client_name".to_string(), + )); + } + } else { + return Err(NeovimError::Api( + "LSP attach notification data is not a map".to_string(), + )); + } + } else { + return Err(NeovimError::Api( + "LSP attach notification missing data".to_string(), + )); + } + } + + debug!("LSP client readiness confirmed"); + Ok(()) + } + + #[instrument(skip(self))] + async fn wait_for_diagnostics( + &self, + buffer_id: Option, + timeout_ms: u64, + ) -> Result, NeovimError> { + debug!( + "Waiting for diagnostics for buffer {:?} with timeout: {}ms", + buffer_id, timeout_ms + ); + + // First try to get diagnostics immediately + match self.get_diagnostics(buffer_id).await { + Ok(diagnostics) if !diagnostics.is_empty() => { + debug!("Found {} diagnostics immediately", diagnostics.len()); + return Ok(diagnostics); + } + Ok(_) => { + // No diagnostics found, wait for diagnostic notification + debug!("No diagnostics found, waiting for notification"); + } + Err(e) => { + debug!("Error getting diagnostics: {}, waiting for notification", e); + } + } + + // Wait for diagnostic notification + let notification = self + .wait_for_notification("NVIM_MCP_DiagnosticsChanged", timeout_ms) + .await?; + + debug!("Received diagnostics notification: {:?}", notification); + + // After notification, try to get diagnostics again + self.get_diagnostics(buffer_id).await + } } #[cfg(test)] @@ -3130,4 +3293,114 @@ mod tests { assert_eq!(notification.args.len(), 1); assert_eq!(notification.args[0].as_str().unwrap(), "async_test_arg"); } + + #[tokio::test] + async fn test_notification_cleanup_expired() { + let tracker = NotificationTracker::new(); + + // Record a notification with a modified timestamp (simulate old notification) + let old_notification = Notification { + name: "old_notification".to_string(), + args: vec![Value::from("old_data")], + timestamp: std::time::SystemTime::now() + - Duration::from_secs(NOTIFICATION_EXPIRY_SECONDS + 1), + }; + + // Manually insert old notification to simulate existing data + { + let mut notifications = tracker.notifications.lock().await; + notifications.push(old_notification); + } + + // Record a fresh notification + tracker + .record_notification( + "fresh_notification".to_string(), + vec![Value::from("fresh_data")], + ) + .await; + + // Check initial state + let (count_before, _) = tracker.get_stats().await; + assert_eq!(count_before, 2); + + // Trigger cleanup + tracker.cleanup_expired_notifications().await; + + // Old notification should be removed, fresh one should remain + let (count_after, _) = tracker.get_stats().await; + assert_eq!(count_after, 1); + + // Verify the remaining notification is the fresh one + let result = tracker + .wait_for_notification("fresh_notification", Duration::from_millis(10)) + .await; + assert!(result.is_ok()); + + // Verify old notification is gone + let result = tracker + .wait_for_notification("old_notification", Duration::from_millis(10)) + .await; + assert!(result.is_err()); + } + + #[tokio::test] + async fn test_notification_cleanup_excess() { + let tracker = NotificationTracker::new(); + + // Record more than MAX_STORED_NOTIFICATIONS + for i in 0..(MAX_STORED_NOTIFICATIONS + 10) { + tracker + .record_notification(format!("notification_{}", i), vec![Value::from(i as i64)]) + .await; + } + + // Get current count + let (count, _) = tracker.get_stats().await; + + // Should be limited to MAX_STORED_NOTIFICATIONS due to automatic cleanup + assert!(count <= MAX_STORED_NOTIFICATIONS); + + // The most recent notifications should still be available + let result = tracker + .wait_for_notification( + &format!("notification_{}", MAX_STORED_NOTIFICATIONS + 9), + Duration::from_millis(10), + ) + .await; + assert!(result.is_ok()); + } + + #[tokio::test] + async fn test_notification_expiry_in_wait() { + let tracker = NotificationTracker::new(); + + // Create an expired notification manually + let expired_notification = Notification { + name: "expired_test".to_string(), + args: vec![Value::from("expired_data")], + timestamp: std::time::SystemTime::now() + - Duration::from_secs(NOTIFICATION_EXPIRY_SECONDS + 1), + }; + + // Manually insert expired notification + { + let mut notifications = tracker.notifications.lock().await; + notifications.push(expired_notification); + } + + // wait_for_notification should not return expired notification + let result = tracker + .wait_for_notification("expired_test", Duration::from_millis(50)) + .await; + + // Should timeout because expired notification is ignored + assert!(result.is_err()); + assert!( + result + .unwrap_err() + .to_string() + .contains("Timeout waiting for notification") + ); + } } diff --git a/src/neovim/integration_tests.rs b/src/neovim/integration_tests.rs index f572361..b5539df 100644 --- a/src/neovim/integration_tests.rs +++ b/src/neovim/integration_tests.rs @@ -1,15 +1,43 @@ use std::fs; -use std::time::Duration; use tempfile::TempDir; -use tokio::time::sleep; use tracing::info; use tracing_test::traced_test; use crate::neovim::client::{DocumentIdentifier, Position, Range}; -use crate::neovim::{NeovimClient, NeovimClientTrait}; +use crate::neovim::{NeovimClient, NeovimClientTrait, NeovimError}; use crate::test_utils::*; +// Test helper functions to reduce boilerplate + +/// Helper function to wait for LSP analysis to complete (LSP ready + diagnostics) +async fn wait_for_lsp_analysis_complete( + client: &mut NeovimClient, + timeout_ms: u64, +) -> Result<(), NeovimError> { + client.wait_for_lsp_ready(None, timeout_ms).await?; + client.wait_for_diagnostics(None, timeout_ms).await?; + Ok(()) +} + +/// Helper for complete LSP setup including autocmd setup and waiting for analysis +async fn setup_lsp_with_analysis( + client: &mut NeovimClient, +) -> Result<(), NeovimError> { + client.setup_autocmd().await?; + wait_for_lsp_analysis_complete(client, 5000).await?; + Ok(()) +} + +/// Helper for LSP setup that only waits for LSP readiness (no diagnostics) +async fn setup_lsp_ready_only( + client: &mut NeovimClient, +) -> Result<(), NeovimError> { + client.setup_autocmd().await?; + client.wait_for_lsp_ready(None, 5000).await?; + Ok(()) +} + #[tokio::test] #[traced_test] async fn test_tcp_connection_lifecycle() { @@ -189,14 +217,10 @@ async fn test_get_vim_diagnostics() { let result = client.connect_path(&ipc_path).await; assert!(result.is_ok(), "Failed to connect to instance"); - // Set up diagnostics and get diagnostics for buffer 0 - let result = client.setup_autocmd().await; - assert!( - result.is_ok(), - "Failed to setup diagnostics autocmd: {result:?}" - ); - - sleep(Duration::from_secs(3)).await; // Allow time for LSP to initialize + // Set up LSP and wait for analysis to complete + setup_lsp_with_analysis(&mut client) + .await + .expect("LSP setup and analysis should complete"); let result = client.get_buffer_diagnostics(0).await; assert!(result.is_ok(), "Failed to get diagnostics: {result:?}"); @@ -225,14 +249,10 @@ async fn test_code_action() { let result = client.connect_path(&ipc_path).await; assert!(result.is_ok(), "Failed to connect to instance"); - // Set up diagnostics and wait for LSP - let result = client.setup_autocmd().await; - assert!( - result.is_ok(), - "Failed to setup diagnostics autocmd: {result:?}" - ); - - sleep(Duration::from_secs(3)).await; // Allow time for LSP to initialize + // Set up LSP and wait for analysis to complete + setup_lsp_with_analysis(&mut client) + .await + .expect("LSP setup and analysis should complete"); let result = client.get_buffer_diagnostics(0).await; assert!(result.is_ok(), "Failed to get diagnostics: {result:?}"); @@ -289,14 +309,10 @@ async fn test_lsp_resolve_code_action() { let result = client.connect_path(&ipc_path).await; assert!(result.is_ok(), "Failed to connect to instance"); - // Set up diagnostics and wait for LSP - let result = client.setup_autocmd().await; - assert!( - result.is_ok(), - "Failed to setup diagnostics autocmd: {result:?}" - ); - - sleep(Duration::from_secs(3)).await; // Allow time for LSP to initialize + // Set up LSP and wait for readiness + setup_lsp_ready_only(&mut client) + .await + .expect("LSP setup should complete"); // Position cursor inside fmt.Println call (line 6, character 6) let result = client @@ -408,14 +424,10 @@ async fn test_lsp_apply_workspace_edit() { let result = client.connect_path(&ipc_path).await; assert!(result.is_ok(), "Failed to connect to instance"); - // Set up diagnostics and wait for LSP - let result = client.setup_autocmd().await; - assert!( - result.is_ok(), - "Failed to setup diagnostics autocmd: {result:?}" - ); - - sleep(Duration::from_secs(3)).await; // Allow time for LSP to initialize + // Set up LSP and wait for analysis to complete + setup_lsp_with_analysis(&mut client) + .await + .expect("LSP setup and analysis should complete"); // Get buffer diagnostics to find modernization opportunities let result = client.get_buffer_diagnostics(0).await; @@ -472,8 +484,7 @@ async fn test_lsp_apply_workspace_edit() { let result = client.execute_lua("vim.cmd('write')").await; assert!(result.is_ok(), "Failed to save buffer: {result:?}"); - // Give some time for the edit and save to be applied - sleep(Duration::from_millis(100)).await; + // File operations should be synchronous in Neovim // Read the modified content to verify the change let modified_content = @@ -544,14 +555,10 @@ func main() { let result = client.connect_path(&ipc_path).await; assert!(result.is_ok(), "Failed to connect to instance"); - // Set up diagnostics and wait for LSP - let result = client.setup_autocmd().await; - assert!( - result.is_ok(), - "Failed to setup diagnostics autocmd: {result:?}" - ); - - sleep(Duration::from_secs(2)).await; // Allow time for LSP to initialize + // Set up LSP and wait for readiness + setup_lsp_ready_only(&mut client) + .await + .expect("LSP setup should complete"); // Get LSP clients let lsp_clients = client.lsp_get_clients().await.unwrap(); @@ -664,14 +671,10 @@ pub fn main() !void { let result = client.connect_path(&ipc_path).await; assert!(result.is_ok(), "Failed to connect to instance"); - // Set up diagnostics and wait for LSP - let result = client.setup_autocmd().await; - assert!( - result.is_ok(), - "Failed to setup diagnostics autocmd: {result:?}" - ); - - sleep(Duration::from_secs(2)).await; // Allow time for LSP to initialize + // Set up LSP and wait for analysis to complete + setup_lsp_with_analysis(&mut client) + .await + .expect("LSP setup and analysis should complete"); // Get LSP clients let lsp_clients = client.lsp_get_clients().await.unwrap(); @@ -782,14 +785,10 @@ func main() { let result = client.connect_path(&ipc_path).await; assert!(result.is_ok(), "Failed to connect to instance"); - // Set up diagnostics and wait for LSP - let result = client.setup_autocmd().await; - assert!( - result.is_ok(), - "Failed to setup diagnostics autocmd: {result:?}" - ); - - sleep(Duration::from_secs(2)).await; // Allow time for LSP to initialize + // Set up LSP and wait for readiness + setup_lsp_ready_only(&mut client) + .await + .expect("LSP setup should complete"); // Get LSP clients let lsp_clients = client.lsp_get_clients().await.unwrap(); @@ -906,14 +905,10 @@ func main() { let result = client.connect_path(&ipc_path).await; assert!(result.is_ok(), "Failed to connect to instance"); - // Set up diagnostics and wait for LSP - let result = client.setup_autocmd().await; - assert!( - result.is_ok(), - "Failed to setup diagnostics autocmd: {result:?}" - ); - - sleep(Duration::from_secs(2)).await; // Allow time for LSP to initialize + // Set up LSP and wait for readiness + setup_lsp_ready_only(&mut client) + .await + .expect("LSP setup should complete"); // Get LSP clients let lsp_clients = client.lsp_get_clients().await.unwrap(); @@ -1008,14 +1003,10 @@ async fn test_lsp_rename_with_prepare() { let result = client.connect_path(&ipc_path).await; assert!(result.is_ok(), "Failed to connect to instance"); - // Set up diagnostics and wait for LSP - let result = client.setup_autocmd().await; - assert!( - result.is_ok(), - "Failed to setup diagnostics autocmd: {result:?}" - ); - - sleep(Duration::from_secs(3)).await; // Allow time for LSP to initialize + // Set up LSP and wait for readiness + setup_lsp_ready_only(&mut client) + .await + .expect("LSP setup should complete"); // Get LSP clients let lsp_clients = client.lsp_get_clients().await.unwrap(); @@ -1061,8 +1052,7 @@ async fn test_lsp_rename_with_prepare() { let result = client.execute_lua("vim.cmd('write')").await; assert!(result.is_ok(), "Failed to save buffer: {result:?}"); - // Give some time for the save operation to complete - sleep(Duration::from_millis(100)).await; + // File operations should be synchronous in Neovim // Read the file content to verify the rename was applied let updated_content = @@ -1111,14 +1101,10 @@ async fn test_lsp_rename_without_prepare() { let result = client.connect_path(&ipc_path).await; assert!(result.is_ok(), "Failed to connect to instance"); - // Set up diagnostics and wait for LSP - let result = client.setup_autocmd().await; - assert!( - result.is_ok(), - "Failed to setup diagnostics autocmd: {result:?}" - ); - - sleep(Duration::from_secs(3)).await; // Allow time for LSP to initialize + // Set up LSP and wait for readiness + setup_lsp_ready_only(&mut client) + .await + .expect("LSP setup should complete"); // Get LSP clients let lsp_clients = client.lsp_get_clients().await.unwrap(); @@ -1164,8 +1150,7 @@ async fn test_lsp_rename_without_prepare() { let result = client.execute_lua("vim.cmd('write')").await; assert!(result.is_ok(), "Failed to save buffer: {result:?}"); - // Give some time for the save operation to complete - sleep(Duration::from_millis(100)).await; + // File operations should be synchronous in Neovim // Read the file content to verify the rename was applied let updated_content = @@ -1244,14 +1229,10 @@ main(); let result = client.connect_path(&ipc_path).await; assert!(result.is_ok(), "Failed to connect to instance"); - // Set up diagnostics and wait for LSP - let result = client.setup_autocmd().await; - assert!( - result.is_ok(), - "Failed to setup diagnostics autocmd: {result:?}" - ); - - sleep(Duration::from_secs(3)).await; // Allow time for LSP to initialize + // Set up LSP and wait for analysis to complete + setup_lsp_with_analysis(&mut client) + .await + .expect("LSP setup and analysis should complete"); (temp_dir, guard, client) } @@ -1341,14 +1322,10 @@ main(); let result = client.connect_path(&ipc_path).await; assert!(result.is_ok(), "Failed to connect to instance"); - // Set up diagnostics and wait for LSP - let result = client.setup_autocmd().await; - assert!( - result.is_ok(), - "Failed to setup diagnostics autocmd: {result:?}" - ); - - sleep(Duration::from_secs(3)).await; // Allow time for LSP to initialize + // Set up LSP and wait for analysis to complete + setup_lsp_with_analysis(&mut client) + .await + .expect("LSP setup and analysis should complete"); use crate::neovim::FormattingOptions; let tab_options = FormattingOptions { diff --git a/src/server/integration_tests.rs b/src/server/integration_tests.rs index 5358769..92c4f12 100644 --- a/src/server/integration_tests.rs +++ b/src/server/integration_tests.rs @@ -1,6 +1,5 @@ use std::path::PathBuf; use std::sync::OnceLock; -use std::time::Duration; use rmcp::{ model::{CallToolRequestParam, ReadResourceRequestParam}, @@ -8,7 +7,7 @@ use rmcp::{ service::ServiceExt, transport::TokioChildProcess, }; -use tokio::{process::Command, time}; +use tokio::process::Command; use tracing::{error, info}; use tracing_test::traced_test; @@ -971,8 +970,6 @@ async fn test_lsp_organize_imports_with_lsp() -> Result<(), Box Result<(), Box Result<(), Box Result<(), Box Result<(), Box, + /// Timeout in milliseconds (default 5000ms) + #[serde(default = "default_timeout")] + pub timeout_ms: u64, +} + +fn default_timeout() -> u64 { + 5000 +} + /// Workspace symbols parameters #[derive(Debug, serde::Deserialize, schemars::JsonSchema)] pub struct WorkspaceSymbolsParams { @@ -510,6 +527,29 @@ impl NeovimMcpServer { )?])) } + #[tool(description = "Wait for LSP client to be ready and attached")] + #[instrument(skip(self))] + pub async fn wait_for_lsp_ready( + &self, + Parameters(WaitForLspReadyRequest { + connection_id, + client_name, + timeout_ms, + }): Parameters, + ) -> Result { + let client = self.get_connection(&connection_id)?; + client + .wait_for_lsp_ready(client_name.as_deref(), timeout_ms) + .await?; + Ok(CallToolResult::success(vec![Content::json( + serde_json::json!({ + "message": "LSP client ready", + "client_name": client_name.unwrap_or_else(|| "any".to_string()), + "timeout_ms": timeout_ms + }), + )?])) + } + #[tool(description = "Get buffer's diagnostics")] #[instrument(skip(self))] pub async fn buffer_diagnostics( From cb81d3bbea3c568e53f2941dc4ce08f68529ad2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Fri, 22 Aug 2025 19:50:00 +0800 Subject: [PATCH 6/7] Add configurable LSP timeout to NeovimClient - Add NeovimClientConfig struct with configurable lsp_timeout_ms (default: 3000ms) - Replace hardcoded 1000ms timeouts with configurable timeout across all LSP operations - Add with_config() method for client configuration - Make unused debugging methods private with allow(dead_code) attributes --- src/neovim/client.rs | 56 ++++++++++++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/src/neovim/client.rs b/src/neovim/client.rs index 1981f10..0cd0745 100644 --- a/src/neovim/client.rs +++ b/src/neovim/client.rs @@ -343,12 +343,14 @@ impl NotificationTracker { } /// Manually trigger cleanup of expired notifications - pub async fn cleanup_expired_notifications(&self) { + #[allow(dead_code)] + pub(crate) async fn cleanup_expired_notifications(&self) { self.cleanup_notifications().await; } /// Get current notification statistics (for debugging/monitoring) - pub async fn get_stats(&self) -> (usize, usize) { + #[allow(dead_code)] + pub(crate) async fn get_stats(&self) -> (usize, usize) { let notifications = self.notifications.lock().await; let wakers = self.notify_wakers.lock().await; (notifications.len(), wakers.len()) @@ -1222,12 +1224,28 @@ pub struct RenameRequestParams { pub new_name: String, } +/// Configuration for Neovim client operations +#[derive(Debug, Clone)] +pub struct NeovimClientConfig { + /// Timeout in milliseconds for LSP operations (default: 3000ms) + pub lsp_timeout_ms: u64, +} + +impl Default for NeovimClientConfig { + fn default() -> Self { + Self { + lsp_timeout_ms: 3000, + } + } +} + pub struct NeovimClient where T: AsyncWrite + Send + 'static, { connection: Option>, notification_tracker: Option, + config: NeovimClientConfig, } #[cfg(unix)] @@ -1361,9 +1379,17 @@ where Self { connection: None, notification_tracker: None, + config: NeovimClientConfig::default(), } } + /// Configure the Neovim client with custom settings + #[allow(dead_code)] + pub fn with_config(mut self, config: NeovimClientConfig) -> Self { + self.config = config; + self + } + #[instrument(skip(self))] async fn get_diagnostics( &self, @@ -1689,7 +1715,7 @@ where }) .unwrap(), ), // params - Value::from(1000), // timeout_ms + Value::from(self.config.lsp_timeout_ms), // timeout_ms Value::from(buffer_id), // bufnr ], ) @@ -1744,7 +1770,7 @@ where }) .unwrap(), ), // params - Value::from(1000), // timeout_ms + Value::from(self.config.lsp_timeout_ms), // timeout_ms Value::from(buffer_id), // bufnr ], ) @@ -1797,7 +1823,7 @@ where Value::from( serde_json::to_string(&DocumentSymbolParams { text_document }).unwrap(), ), // params - Value::from(1000), // timeout_ms + Value::from(self.config.lsp_timeout_ms), // timeout_ms Value::from(buffer_id), // bufnr ], ) @@ -1847,7 +1873,7 @@ where }) .unwrap(), ), // params - Value::from(1000), // timeout_ms + Value::from(self.config.lsp_timeout_ms), // timeout_ms ], ) .await @@ -1910,7 +1936,7 @@ where }) .unwrap(), ), // params - Value::from(1000), // timeout_ms + Value::from(self.config.lsp_timeout_ms), // timeout_ms Value::from(buffer_id), // bufnr ], ) @@ -1967,7 +1993,7 @@ where }) .unwrap(), ), // params - Value::from(1000), // timeout_ms + Value::from(self.config.lsp_timeout_ms), // timeout_ms ], ) .await @@ -2020,7 +2046,7 @@ where }) .unwrap(), ), // params - Value::from(1000), // timeout_ms + Value::from(self.config.lsp_timeout_ms), // timeout_ms ], ) .await @@ -2073,7 +2099,7 @@ where }) .unwrap(), ), // params - Value::from(1000), // timeout_ms + Value::from(self.config.lsp_timeout_ms), // timeout_ms ], ) .await @@ -2126,7 +2152,7 @@ where }) .unwrap(), ), // params - Value::from(1000), // timeout_ms + Value::from(self.config.lsp_timeout_ms), // timeout_ms ], ) .await @@ -2274,7 +2300,7 @@ where }) .unwrap(), ), - Value::from(1000), + Value::from(self.config.lsp_timeout_ms), Value::from(buffer_id), ], ) @@ -2392,7 +2418,7 @@ where }) .unwrap(), ), - Value::from(1000), // timeout_ms + Value::from(self.config.lsp_timeout_ms), // timeout_ms ], ) .await @@ -2456,7 +2482,7 @@ where }) .unwrap(), ), - Value::from(1000), // timeout_ms + Value::from(self.config.lsp_timeout_ms), // timeout_ms ], ) .await @@ -2526,7 +2552,7 @@ where }) .unwrap(), ), // params - Value::from(1000), // timeout_ms + Value::from(self.config.lsp_timeout_ms), // timeout_ms Value::from(buffer_id), // bufnr ], ) From d34d32dae2c956b0cc77da15b4cc8cfcb0451f4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Fri, 22 Aug 2025 20:54:31 +0800 Subject: [PATCH 7/7] Update documentation for LSP readiness and configuration features - Add wait_for_lsp_ready tool documentation across all files - Update tool count from 23 to 24 tools - Document configurable LSP timeout settings - Include notification tracking system improvements - Add enhanced test reliability information --- CHANGELOG.md | 16 ++++++++++++++++ CLAUDE.md | 43 +++++++++++++++++++++++++------------------ README.md | 7 ++++++- docs/instructions.md | 12 +++++++++++- 4 files changed, 58 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5253229..87e4179 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,22 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### New Features + +- **LSP Readiness Tool**: Added `wait_for_lsp_ready` tool for ensuring LSP client + readiness before performing operations, improving reliability of LSP workflows + +### Improved + +- **Configurable LSP Timeout**: Added `NeovimClientConfig` with configurable LSP + timeout settings (default: 3000ms) for better control over LSP operation timing +- **Enhanced Notification Tracking**: Comprehensive notification tracking system + for robust LSP synchronization and event handling +- **Autocmd Setup**: Unified autocmd setup replacing diagnostics-specific + implementation with more comprehensive event handling +- **Test Performance**: Optimized integration tests with better timing and + one-time binary compilation for improved CI performance + ## [v0.5.0] - 2025-08-20 ### Fixed diff --git a/CLAUDE.md b/CLAUDE.md index a380bac..2900cd0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -134,6 +134,9 @@ The codebase follows a modular architecture with clear separation of concerns: - Supports both TCP and Unix socket/named pipe connections - Provides high-level operations: buffer management, diagnostics, LSP integration - Handles Lua code execution and autocmd setup + - Includes configurable LSP timeout settings via `NeovimClientConfig` + - Features comprehensive notification tracking system for LSP synchronization + - Provides methods for waiting on LSP readiness and diagnostic availability - **`src/neovim/connection.rs`**: Connection management layer - Wraps `nvim-rs` client with lifecycle management @@ -210,7 +213,7 @@ This modular architecture provides several advantages: ### Available MCP Tools -The server provides these 23 tools (implemented with `#[tool]` attribute): +The server provides these 24 tools (implemented with `#[tool]` attribute): **Connection Management:** @@ -224,30 +227,31 @@ The server provides these 23 tools (implemented with `#[tool]` attribute): 1. **`list_buffers`**: List all open buffers for specific connection 2. **`exec_lua`**: Execute arbitrary Lua code in specific Neovim instance -3. **`buffer_diagnostics`**: Get diagnostics for specific buffer on specific connection -4. **`lsp_clients`**: Get workspace LSP clients for specific connection -5. **`lsp_workspace_symbols`**: Search workspace symbols by query on specific +3. **`wait_for_lsp_ready`**: Wait for LSP client to be ready and attached with timeout +4. **`buffer_diagnostics`**: Get diagnostics for specific buffer on specific connection +5. **`lsp_clients`**: Get workspace LSP clients for specific connection +6. **`lsp_workspace_symbols`**: Search workspace symbols by query on specific connection -6. **`lsp_code_actions`**: Get LSP code actions with universal document +7. **`lsp_code_actions`**: Get LSP code actions with universal document identification (supports buffer IDs, project-relative paths, and absolute paths) -7. **`lsp_hover`**: Get LSP hover information with universal document +8. **`lsp_hover`**: Get LSP hover information with universal document identification (supports buffer IDs, project-relative paths, and absolute paths) -8. **`lsp_document_symbols`**: Get document symbols with universal document +9. **`lsp_document_symbols`**: Get document symbols with universal document identification (supports buffer IDs, project-relative paths, and absolute paths) -9. **`lsp_references`**: Get LSP references with universal document +10. **`lsp_references`**: Get LSP references with universal document identification (supports buffer IDs, project-relative paths, and absolute paths) -10. **`lsp_resolve_code_action`**: Resolve code actions that may have +11. **`lsp_resolve_code_action`**: Resolve code actions that may have incomplete data -11. **`lsp_apply_edit`**: Apply workspace edits using Neovim's LSP utility +12. **`lsp_apply_edit`**: Apply workspace edits using Neovim's LSP utility functions -12. **`lsp_definition`**: Get LSP definition with universal document identification -13. **`lsp_type_definition`**: Get LSP type definition with universal document identification -14. **`lsp_implementations`**: Get LSP implementations with universal document identification -15. **`lsp_declaration`**: Get LSP declaration with universal document identification -16. **`lsp_rename`**: Rename symbol across workspace using LSP -17. **`lsp_formatting`**: Format document using LSP with optional auto-apply -18. **`lsp_range_formatting`**: Format a specific range in a document using LSP -19. **`lsp_organize_imports`**: Sort and organize imports using LSP with +13. **`lsp_definition`**: Get LSP definition with universal document identification +14. **`lsp_type_definition`**: Get LSP type definition with universal document identification +15. **`lsp_implementations`**: Get LSP implementations with universal document identification +16. **`lsp_declaration`**: Get LSP declaration with universal document identification +17. **`lsp_rename`**: Rename symbol across workspace using LSP +18. **`lsp_formatting`**: Format document using LSP with optional auto-apply +19. **`lsp_range_formatting`**: Format a specific range in a document using LSP +20. **`lsp_organize_imports`**: Sort and organize imports using LSP with auto-apply by default ### Universal Document Identifier System @@ -505,6 +509,9 @@ the new `lua_tools.rs` module: lsp_apply_edit - **Test data**: Includes Go source files and LSP configuration for realistic testing scenarios +- **Enhanced reliability**: Robust LSP synchronization with notification tracking +- **Optimized timing**: Better test performance with improved setup and teardown +- **Notification testing**: Unit tests for notification tracking system ## Error Handling diff --git a/README.md b/README.md index 4883a52..12f1b08 100644 --- a/README.md +++ b/README.md @@ -219,7 +219,7 @@ and resources - only the transport layer changes. ## Available Tools -The server provides 23 MCP tools for interacting with Neovim: +The server provides 24 MCP tools for interacting with Neovim: ### Connection Management @@ -366,6 +366,11 @@ providing enhanced flexibility for code analysis and navigation. - **`exec_lua`**: Execute Lua code in Neovim - Parameters: `connection_id` (string), `code` (string) - Lua code to execute +- **`wait_for_lsp_ready`**: Wait for LSP client to be ready and attached + - Parameters: `connection_id` (string), `client_name` (string, optional), + `timeout_ms` (number, optional, default: 5000ms) + - Returns: Success confirmation with LSP client readiness status + ### Complete LSP Code Action Workflow The server now supports the full LSP code action lifecycle: diff --git a/docs/instructions.md b/docs/instructions.md index a7e0860..72a5eb6 100644 --- a/docs/instructions.md +++ b/docs/instructions.md @@ -4,7 +4,7 @@ ### Tools -The server provides 23 MCP tools for interacting with Neovim instances: +The server provides 24 MCP tools for interacting with Neovim instances: #### Connection Management @@ -48,6 +48,16 @@ All tools below require a `connection_id` parameter from connection establishmen - **Returns**: Object with execution result - **Usage**: Run Neovim commands, get editor state, or modify configuration +- **`wait_for_lsp_ready`**: Wait for LSP client to be ready and attached + - **Parameters**: + - `connection_id` (string): Target Neovim instance ID + - `client_name` (string, optional): Specific LSP client name to wait for + (waits for any if not specified) + - `timeout_ms` (number, optional): Timeout in milliseconds (default: 5000ms) + - **Returns**: Success confirmation with client readiness status + - **Usage**: Ensure LSP client is ready before performing LSP operations + for reliable results + - **`buffer_diagnostics`**: Get diagnostics for specific buffer - **Parameters**: - `connection_id` (string): Target Neovim instance ID