diff --git a/CHANGELOG.md b/CHANGELOG.md index b2604a5..c84ae1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **MCP resources** — expose LSP diagnostics as subscribable MCP resources under the `lsp-diagnostics:///` URI scheme; clients can call `list_resources`, `read_resource`, `subscribe`, and `unsubscribe` (#115) +- **Diagnostics push notifications** — background `diagnostics_pump` tasks relay `textDocument/publishDiagnostics` LSP notifications to subscribed MCP clients via `notifications/resources/updated` +- **RFC-3986 URI codec** — `bridge::resources` module with percent-encoding via `url::Url::from_file_path`; empty-authority injection is rejected to prevent UNC-path attacks on Windows +- **Subscription cap** — `ResourceSubscriptions` enforces a `MAX_SUBSCRIPTIONS = 1_000` limit per session to guard against memory exhaustion +- **MCP tools** — `get_signature_help` (`textDocument/signatureHelp`), `go_to_implementation` (`textDocument/implementation`), `go_to_type_definition` (`textDocument/typeDefinition`), and `get_inlay_hints` (`textDocument/inlayHint`) tools exposing LSP 3.6/3.15/3.17 capabilities (#116) + ### Changed - **LSP API** — Breaking change: `InboundMessage` is now non-exhaustive and includes a server-request variant for LSP server-to-client JSON-RPC requests. Downstream exhaustive matches must include a wildcard arm. diff --git a/crates/mcpls-core/src/bridge/mod.rs b/crates/mcpls-core/src/bridge/mod.rs index e26eb6d..e6830bb 100644 --- a/crates/mcpls-core/src/bridge/mod.rs +++ b/crates/mcpls-core/src/bridge/mod.rs @@ -5,6 +5,7 @@ mod encoding; mod notifications; +pub mod resources; mod state; mod translator; @@ -12,7 +13,8 @@ pub use encoding::{PositionEncoding, lsp_to_mcp_position, mcp_to_lsp_position}; pub use notifications::{ DiagnosticInfo, LogEntry, LogLevel, MessageType, NotificationCache, ServerMessage, }; -pub use state::{DocumentState, DocumentTracker}; +pub use resources::ResourceSubscriptions; +pub use state::{DocumentState, DocumentTracker, path_to_uri, uri_to_path}; pub use translator::{ Completion, CompletionsResult, DefinitionResult, Diagnostic, DiagnosticSeverity, DiagnosticsResult, DocumentChanges, DocumentSymbolsResult, FormatDocumentResult, HoverResult, diff --git a/crates/mcpls-core/src/bridge/resources.rs b/crates/mcpls-core/src/bridge/resources.rs new file mode 100644 index 0000000..e27eae0 --- /dev/null +++ b/crates/mcpls-core/src/bridge/resources.rs @@ -0,0 +1,325 @@ +//! MCP resource URI codec and subscription tracking for LSP diagnostics. +//! +//! Resources in mcpls use the `lsp-diagnostics:///` scheme (RFC 3986 compliant, +//! empty authority, percent-encoded path). Each resource corresponds to a single +//! file whose diagnostics are cached from LSP `textDocument/publishDiagnostics` +//! notifications. + +use std::collections::HashSet; +use std::path::{Path, PathBuf}; + +use thiserror::Error; +use tokio::sync::RwLock; +use url::Url; + +/// URI scheme used for diagnostic resources. +const SCHEME: &str = "lsp-diagnostics"; + +/// Full scheme + authority prefix (`scheme://`). +/// +/// Three-slash form (`lsp-diagnostics:///`) is produced by appending an empty +/// authority and the absolute path: `{PREFIX}{path}`. +const PREFIX: &str = "lsp-diagnostics://"; + +/// Maximum number of resource URIs a single client session may subscribe to. +/// +/// Guards against memory exhaustion from a misbehaving or adversarial client. +pub const MAX_SUBSCRIPTIONS: usize = 1_000; + +/// Errors produced by the resource URI codec. +#[derive(Debug, Error)] +pub enum ResourceUriError { + /// The path is relative or contains non-UTF-8 components. + #[error("path must be absolute and valid UTF-8: {0}")] + InvalidPath(String), + + /// The URI has the wrong scheme or malformed structure. + #[error("expected '{SCHEME}:///' prefix in URI: {0}")] + InvalidScheme(String), + + /// The URI path could not be decoded to a filesystem path. + #[error("failed to decode URI to filesystem path: {0}")] + DecodeFailed(String), +} + +/// Encode an absolute filesystem path into a `lsp-diagnostics:///…` resource URI. +/// +/// Percent-encoding is delegated to [`url::Url::from_file_path`], which +/// handles spaces, unicode, `%`, `?`, `#`, and platform separators correctly. +/// +/// # Errors +/// +/// Returns [`ResourceUriError::InvalidPath`] if the path is relative or +/// cannot be expressed as a valid file URI. +/// +/// # Examples +/// +/// ``` +/// use std::path::Path; +/// use mcpls_core::bridge::resources::make_uri; +/// +/// let uri = make_uri(Path::new("/home/user/main.rs")).unwrap(); +/// assert!(uri.starts_with("lsp-diagnostics:///")); +/// ``` +pub fn make_uri(path: &Path) -> Result { + let file_url = Url::from_file_path(path) + .map_err(|()| ResourceUriError::InvalidPath(path.display().to_string()))?; + + // Replace the "file" scheme with our custom scheme while keeping the + // already-percent-encoded path and authority (empty) components. + let uri = format!("{SCHEME}://{}", &file_url[url::Position::BeforeHost..]); + Ok(uri) +} + +/// Decode a `lsp-diagnostics:///…` resource URI back to an absolute filesystem path. +/// +/// # Errors +/// +/// Returns an error if the URI does not start with the expected scheme, +/// or if the percent-encoded path cannot be mapped to a filesystem path. +/// +/// # Examples +/// +/// ``` +/// use std::path::Path; +/// use mcpls_core::bridge::resources::{make_uri, parse_uri}; +/// +/// let path = Path::new("/home/user/main.rs"); +/// let uri = make_uri(path).unwrap(); +/// let recovered = parse_uri(&uri).unwrap(); +/// assert_eq!(recovered, path); +/// ``` +pub fn parse_uri(uri: &str) -> Result { + if !uri.starts_with(PREFIX) { + return Err(ResourceUriError::InvalidScheme(uri.to_string())); + } + + // Require empty authority: the character immediately after `://` must be `/`. + // This blocks `lsp-diagnostics://evil-host/path` → UNC path on Windows. + let after_prefix = &uri[PREFIX.len()..]; + if !after_prefix.starts_with('/') { + return Err(ResourceUriError::InvalidScheme(format!( + "non-empty authority in URI: {uri}" + ))); + } + + let file_uri = format!("file://{after_prefix}"); + let url = Url::parse(&file_uri).map_err(|e| ResourceUriError::DecodeFailed(e.to_string()))?; + + url.to_file_path() + .map_err(|()| ResourceUriError::DecodeFailed(file_uri)) +} + +/// Tracks which MCP resource URIs the client has subscribed to. +/// +/// The hot read path (pump tasks checking before sending notifications) uses +/// a `RwLock` so concurrent readers do not block each other. +#[derive(Debug)] +pub struct ResourceSubscriptions(RwLock>); + +impl Default for ResourceSubscriptions { + fn default() -> Self { + Self::new() + } +} + +impl ResourceSubscriptions { + /// Create an empty subscription set. + #[must_use] + pub fn new() -> Self { + Self(RwLock::new(HashSet::new())) + } + + /// Add a URI to the subscription set. + /// + /// Returns `Ok(true)` if newly inserted, `Ok(false)` if already present. + /// Returns `Err` if the subscription set has reached [`MAX_SUBSCRIPTIONS`]. + /// + /// # Errors + /// + /// Returns an error string when the cap is exceeded. + pub async fn subscribe(&self, uri: String) -> Result { + let mut set = self.0.write().await; + if !set.contains(&uri) && set.len() >= MAX_SUBSCRIPTIONS { + return Err(format!("subscription limit of {MAX_SUBSCRIPTIONS} reached")); + } + Ok(set.insert(uri)) + } + + /// Check whether the subscription set is empty. + /// + /// Used as a fast path in the diagnostics pump to skip URI construction + /// when no client has subscribed yet. + pub async fn is_empty(&self) -> bool { + self.0.read().await.is_empty() + } + + /// Remove a URI from the subscription set. + /// + /// Returns `true` if the URI was present and removed. + pub async fn unsubscribe(&self, uri: &str) -> bool { + self.0.write().await.remove(uri) + } + + /// Check if a URI is currently subscribed. + pub async fn contains(&self, uri: &str) -> bool { + self.0.read().await.contains(uri) + } + + /// Return a snapshot of all subscribed URIs (primarily for tests). + pub async fn snapshot(&self) -> Vec { + self.0.read().await.iter().cloned().collect() + } +} + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used)] +mod tests { + use super::*; + + // ------------------------------------------------------------------ + // URI codec + // ------------------------------------------------------------------ + + #[test] + fn test_make_uri_rejects_relative_path() { + let result = make_uri(Path::new("relative/path.rs")); + assert!(result.is_err()); + } + + #[test] + fn test_parse_uri_rejects_wrong_scheme() { + let result = parse_uri("file:///home/user/main.rs"); + assert!(result.is_err()); + } + + #[test] + fn test_parse_uri_rejects_http_scheme() { + let result = parse_uri("https://example.com/file.rs"); + assert!(result.is_err()); + } + + #[cfg(unix)] + #[test] + fn test_make_uri_simple_path() { + let uri = make_uri(Path::new("/home/user/main.rs")).unwrap(); + assert_eq!(uri, "lsp-diagnostics:///home/user/main.rs"); + } + + #[cfg(unix)] + #[test] + fn test_make_uri_scheme_prefix() { + let uri = make_uri(Path::new("/tmp/file.rs")).unwrap(); + assert!(uri.starts_with("lsp-diagnostics:///")); + } + + #[cfg(unix)] + #[test] + fn test_parse_uri_simple() { + let path = PathBuf::from("/home/user/main.rs"); + let uri = make_uri(&path).unwrap(); + let recovered = parse_uri(&uri).unwrap(); + assert_eq!(recovered, path); + } + + /// Round-trip: paths with spaces, unicode, `%`, `?`, `#`. + #[cfg(unix)] + #[test] + fn test_round_trip_special_chars() { + let paths = [ + "/home/user/my file.rs", + "/tmp/café/main.rs", + "/data/100%/test.rs", + "/workspace/query?param/file.rs", + "/repo/branch#fragment/src.rs", + "/путь/к/файлу.rs", + ]; + + for raw in &paths { + let path = PathBuf::from(raw); + let uri = make_uri(&path).expect(raw); + assert!( + uri.starts_with("lsp-diagnostics:///"), + "URI should start with correct scheme: {uri}" + ); + let recovered = parse_uri(&uri).expect(&uri); + assert_eq!(recovered, path, "Round-trip failed for: {raw}"); + } + } + + /// Snapshot test: verify the on-wire form uses three slashes and percent-encoding. + #[cfg(unix)] + #[test] + fn test_wire_format_percent_encoded() { + let path = Path::new("/home/user/my file.rs"); + let uri = make_uri(path).unwrap(); + // Space must be percent-encoded as %20 + assert!(uri.contains("%20"), "Expected %20 in: {uri}"); + assert!(uri.starts_with("lsp-diagnostics:///")); + } + + // ------------------------------------------------------------------ + // ResourceSubscriptions + // ------------------------------------------------------------------ + + #[tokio::test] + async fn test_subscribe_and_contains() { + let subs = ResourceSubscriptions::new(); + let uri = "lsp-diagnostics:///home/user/main.rs".to_string(); + + assert!(!subs.contains(&uri).await); + assert!(subs.subscribe(uri.clone()).await.unwrap()); + assert!(subs.contains(&uri).await); + } + + #[tokio::test] + async fn test_subscribe_duplicate_returns_false() { + let subs = ResourceSubscriptions::new(); + let uri = "lsp-diagnostics:///tmp/file.rs".to_string(); + assert!(subs.subscribe(uri.clone()).await.unwrap()); + assert!(!subs.subscribe(uri).await.unwrap()); + } + + #[tokio::test] + async fn test_unsubscribe() { + let subs = ResourceSubscriptions::new(); + let uri = "lsp-diagnostics:///tmp/file.rs".to_string(); + subs.subscribe(uri.clone()).await.unwrap(); + assert!(subs.unsubscribe(&uri).await); + assert!(!subs.contains(&uri).await); + } + + #[tokio::test] + async fn test_unsubscribe_nonexistent_returns_false() { + let subs = ResourceSubscriptions::new(); + assert!(!subs.unsubscribe("lsp-diagnostics:///nonexistent.rs").await); + } + + #[tokio::test] + async fn test_subscribe_cap_exceeded() { + let subs = ResourceSubscriptions::new(); + for i in 0..MAX_SUBSCRIPTIONS { + subs.subscribe(format!("lsp-diagnostics:///file{i}.rs")) + .await + .unwrap(); + } + let result = subs + .subscribe("lsp-diagnostics:///overflow.rs".to_string()) + .await; + assert!(result.is_err()); + } + + #[tokio::test] + async fn test_snapshot() { + let subs = ResourceSubscriptions::new(); + subs.subscribe("lsp-diagnostics:///a.rs".to_string()) + .await + .unwrap(); + subs.subscribe("lsp-diagnostics:///b.rs".to_string()) + .await + .unwrap(); + let mut snap = subs.snapshot().await; + snap.sort(); + assert_eq!(snap, ["lsp-diagnostics:///a.rs", "lsp-diagnostics:///b.rs"]); + } +} diff --git a/crates/mcpls-core/src/bridge/state.rs b/crates/mcpls-core/src/bridge/state.rs index d681861..64fec01 100644 --- a/crates/mcpls-core/src/bridge/state.rs +++ b/crates/mcpls-core/src/bridge/state.rs @@ -6,6 +6,7 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; use lsp_types::{DidOpenTextDocumentParams, TextDocumentItem, Uri}; +use url::Url; use crate::error::{Error, Result}; use crate::lsp::LspClient; @@ -153,6 +154,11 @@ impl DocumentTracker { self.documents.drain().map(|(_, state)| state).collect() } + /// Iterate over the filesystem paths of all currently open documents. + pub fn open_paths(&self) -> impl Iterator { + self.documents.keys().map(PathBuf::as_path) + } + /// Ensure a document is open, opening it lazily if necessary. /// /// If the document is already open, returns its URI immediately. @@ -199,6 +205,11 @@ impl DocumentTracker { } /// Convert a file path to a URI. +/// +/// # Panics +/// +/// Panics if the path cannot be represented as a `file://` URI. This should +/// not occur for valid absolute paths. #[must_use] pub fn path_to_uri(path: &Path) -> Uri { // Convert path to file:// URI string and parse @@ -212,6 +223,24 @@ pub fn path_to_uri(path: &Path) -> Uri { uri_string.parse().expect("failed to create URI from path") } +/// Convert an LSP `file://` URI to an absolute filesystem path. +/// +/// Returns `None` if the URI is not a valid `file://` URI, uses a non-file +/// scheme, or contains percent-encoding that cannot map to a valid path. +#[must_use] +pub fn uri_to_path(uri: &Uri) -> Option { + let url = Url::parse(uri.as_str()).ok()?; + if url.scheme() != "file" { + return None; + } + // Reject authority-bearing file URIs (e.g. `file://server/share`) to + // avoid UNC path confusion on Windows. + if !url.host_str().unwrap_or("").is_empty() { + return None; + } + url.to_file_path().ok() +} + /// Detect the language ID from a file path. /// /// Consults the extension map to determine the language ID for a file. @@ -800,4 +829,73 @@ mod tests { // Lowercase .nu should not match uppercase "NU" in map assert_eq!(detect_language(Path::new("script.nu"), &map), "plaintext"); } + + // ------------------------------------------------------------------ + // uri_to_path + // ------------------------------------------------------------------ + + #[cfg(unix)] + #[test] + fn test_uri_to_path_file_scheme() { + let uri: Uri = "file:///home/user/main.rs".parse().unwrap(); + let path = uri_to_path(&uri).unwrap(); + assert_eq!(path, PathBuf::from("/home/user/main.rs")); + } + + #[test] + fn test_uri_to_path_non_file_scheme_returns_none() { + let uri: Uri = "https://example.com/file.rs".parse().unwrap(); + assert!(uri_to_path(&uri).is_none()); + } + + #[test] + fn test_uri_to_path_lsp_diagnostics_scheme_returns_none() { + // Custom scheme must not be decoded by uri_to_path. + let uri: Uri = "lsp-diagnostics:///home/user/main.rs".parse().unwrap(); + assert!(uri_to_path(&uri).is_none()); + } + + #[test] + fn test_uri_to_path_with_authority_returns_none() { + // Authority-bearing file URIs must be rejected (UNC path defence). + // lsp_types::Uri may or may not accept this string; either way + // uri_to_path should return None. + let result = "file://server/share/path.rs" + .parse::() + .ok() + .and_then(|u| uri_to_path(&u)); + assert!(result.is_none()); + } + + // ------------------------------------------------------------------ + // open_paths + // ------------------------------------------------------------------ + + #[test] + fn test_open_paths_empty_tracker() { + let tracker = DocumentTracker::new(ResourceLimits::default(), HashMap::new()); + assert_eq!(tracker.open_paths().count(), 0); + } + + #[test] + fn test_open_paths_populated_tracker() { + let mut map = HashMap::new(); + map.insert("rs".to_string(), "rust".to_string()); + let mut tracker = DocumentTracker::new(ResourceLimits::default(), map); + tracker.open(PathBuf::from("/a.rs"), String::new()).unwrap(); + tracker.open(PathBuf::from("/b.rs"), String::new()).unwrap(); + let mut paths: Vec<_> = tracker.open_paths().collect(); + paths.sort(); + assert_eq!(paths, [Path::new("/a.rs"), Path::new("/b.rs")]); + } + + #[test] + fn test_open_paths_after_close() { + let mut map = HashMap::new(); + map.insert("rs".to_string(), "rust".to_string()); + let mut tracker = DocumentTracker::new(ResourceLimits::default(), map); + tracker.open(PathBuf::from("/a.rs"), String::new()).unwrap(); + tracker.close(Path::new("/a.rs")); + assert_eq!(tracker.open_paths().count(), 0); + } } diff --git a/crates/mcpls-core/src/bridge/translator.rs b/crates/mcpls-core/src/bridge/translator.rs index e334b2b..842358c 100644 --- a/crates/mcpls-core/src/bridge/translator.rs +++ b/crates/mcpls-core/src/bridge/translator.rs @@ -9,9 +9,11 @@ use lsp_types::{ CallHierarchyPrepareParams as LspCallHierarchyPrepareParams, CompletionParams, CompletionTriggerKind, DocumentFormattingParams, DocumentSymbol, DocumentSymbolParams, FormattingOptions, GotoDefinitionParams, Hover, HoverContents, HoverParams as LspHoverParams, - MarkedString, PartialResultParams, ReferenceContext, ReferenceParams, - RenameParams as LspRenameParams, TextDocumentIdentifier, TextDocumentPositionParams, - WorkDoneProgressParams, WorkspaceEdit, WorkspaceSymbolParams as LspWorkspaceSymbolParams, + InlayHintLabel, InlayHintParams, MarkedString, PartialResultParams, ReferenceContext, + ReferenceParams, RenameParams as LspRenameParams, + SignatureHelpParams as LspSignatureHelpParams, TextDocumentIdentifier, + TextDocumentPositionParams, WorkDoneProgressParams, WorkspaceEdit, + WorkspaceSymbolParams as LspWorkspaceSymbolParams, }; use serde::{Deserialize, Serialize}; use tokio::time::Duration; @@ -417,6 +419,76 @@ pub struct ServerMessagesResult { pub messages: Vec, } +/// A single parameter in a signature. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SignatureParameter { + /// Label of the parameter. + pub label: String, + /// Optional documentation for the parameter. + #[serde(skip_serializing_if = "Option::is_none")] + pub documentation: Option, +} + +/// A single signature overload. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SignatureInfo { + /// Full label of the signature. + pub label: String, + /// Optional documentation for the signature. + #[serde(skip_serializing_if = "Option::is_none")] + pub documentation: Option, + /// Parameters of the signature. + pub parameters: Vec, +} + +/// Result of a signature help request. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SignatureHelpResult { + /// Available signatures. + pub signatures: Vec, + /// Index of the active signature. + #[serde(skip_serializing_if = "Option::is_none")] + pub active_signature: Option, + /// Index of the active parameter within the active signature. + #[serde(skip_serializing_if = "Option::is_none")] + pub active_parameter: Option, +} + +/// Result of a go-to-implementation or go-to-type-definition request. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct LocationsResult { + /// Locations found. + pub locations: Vec, +} + +/// A single inlay hint entry. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct InlayHintEntry { + /// Position of the hint (1-based MCP). + pub position: Position2D, + /// Label text for the hint. + pub label: String, + /// Hint kind (1 = Type, 2 = Parameter). + #[serde(skip_serializing_if = "Option::is_none")] + pub kind: Option, + /// Whether to add a space before the hint. + #[serde(skip_serializing_if = "Option::is_none")] + pub padding_left: Option, + /// Whether to add a space after the hint. + #[serde(skip_serializing_if = "Option::is_none")] + pub padding_right: Option, + /// Tooltip text. + #[serde(skip_serializing_if = "Option::is_none")] + pub tooltip: Option, +} + +/// Result of an inlay hints request. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct InlayHintsResult { + /// List of inlay hints. + pub hints: Vec, +} + /// Maximum allowed position value for validation. const MAX_POSITION_VALUE: u32 = 1_000_000; /// Maximum allowed range size in lines. @@ -428,7 +500,7 @@ impl Translator { /// # Errors /// /// Returns `Error::PathOutsideWorkspace` if the path is outside all workspace roots. - fn validate_path(&self, path: &Path) -> Result { + pub(crate) fn validate_path(&self, path: &Path) -> Result { let canonical = path.canonicalize().map_err(|e| Error::FileIo { path: path.to_path_buf(), source: e, @@ -1442,9 +1514,282 @@ impl Translator { let messages: Vec<_> = all_messages.iter().take(limit).cloned().collect(); Ok(ServerMessagesResult { messages }) } + + /// Handle signature help request (`textDocument/signatureHelp`). + /// + /// Returns parameter signatures and documentation while typing a function call. + /// `context` is omitted (None) — the server infers trigger state from position. + /// + /// # Errors + /// + /// Returns an error if the LSP request fails or the file cannot be opened. + pub async fn handle_signature_help( + &mut self, + file_path: String, + line: u32, + character: u32, + ) -> Result { + let path = PathBuf::from(&file_path); + let validated_path = self.validate_path(&path)?; + let client = self.get_client_for_file(&validated_path)?; + let uri = self + .document_tracker + .ensure_open(&validated_path, &client) + .await?; + let lsp_position = mcp_to_lsp_position(line, character); + + let params = LspSignatureHelpParams { + text_document_position_params: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { uri }, + position: lsp_position, + }, + work_done_progress_params: WorkDoneProgressParams::default(), + context: None, + }; + + let timeout_duration = Duration::from_secs(30); + let response: Option = client + .request("textDocument/signatureHelp", params, timeout_duration) + .await?; + + let result = match response { + Some(sig_help) => SignatureHelpResult { + signatures: sig_help + .signatures + .into_iter() + .map(|sig| SignatureInfo { + label: sig.label, + documentation: sig.documentation.map(extract_documentation), + parameters: sig + .parameters + .unwrap_or_default() + .into_iter() + .map(|p| SignatureParameter { + label: match p.label { + lsp_types::ParameterLabel::Simple(s) => s, + lsp_types::ParameterLabel::LabelOffsets([start, end]) => { + format!("[{start},{end}]") + } + }, + documentation: p.documentation.map(extract_documentation), + }) + .collect(), + }) + .collect(), + active_signature: sig_help.active_signature, + active_parameter: sig_help.active_parameter, + }, + None => SignatureHelpResult { + signatures: vec![], + active_signature: None, + active_parameter: None, + }, + }; + + Ok(result) + } + + /// Handle go-to-implementation request (`textDocument/implementation`). + /// + /// Returns the locations of trait method or interface member implementations. + /// + /// # Errors + /// + /// Returns an error if the LSP request fails or the file cannot be opened. + pub async fn handle_implementation( + &mut self, + file_path: String, + line: u32, + character: u32, + ) -> Result { + let path = PathBuf::from(&file_path); + let validated_path = self.validate_path(&path)?; + let client = self.get_client_for_file(&validated_path)?; + let uri = self + .document_tracker + .ensure_open(&validated_path, &client) + .await?; + let lsp_position = mcp_to_lsp_position(line, character); + + let params = GotoDefinitionParams { + text_document_position_params: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { uri }, + position: lsp_position, + }, + work_done_progress_params: WorkDoneProgressParams::default(), + partial_result_params: PartialResultParams::default(), + }; + + let timeout_duration = Duration::from_secs(30); + let response: Option = client + .request("textDocument/implementation", params, timeout_duration) + .await?; + + Ok(LocationsResult { + locations: goto_response_to_locations(response), + }) + } + + /// Handle go-to-type-definition request (`textDocument/typeDefinition`). + /// + /// Returns the type definition location of the expression at position. Distinct + /// from go-to-definition for variable bindings where definition and type differ. + /// + /// # Errors + /// + /// Returns an error if the LSP request fails or the file cannot be opened. + pub async fn handle_type_definition( + &mut self, + file_path: String, + line: u32, + character: u32, + ) -> Result { + let path = PathBuf::from(&file_path); + let validated_path = self.validate_path(&path)?; + let client = self.get_client_for_file(&validated_path)?; + let uri = self + .document_tracker + .ensure_open(&validated_path, &client) + .await?; + let lsp_position = mcp_to_lsp_position(line, character); + + let params = GotoDefinitionParams { + text_document_position_params: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { uri }, + position: lsp_position, + }, + work_done_progress_params: WorkDoneProgressParams::default(), + partial_result_params: PartialResultParams::default(), + }; + + let timeout_duration = Duration::from_secs(30); + let response: Option = client + .request("textDocument/typeDefinition", params, timeout_duration) + .await?; + + Ok(LocationsResult { + locations: goto_response_to_locations(response), + }) + } + + /// Handle inlay hints request (`textDocument/inlayHint`). + /// + /// Returns inferred type and parameter annotations the editor would render inline. + /// Output positions are in MCP 1-based form. + /// + /// # Errors + /// + /// Returns an error if the LSP request fails or the file cannot be opened. + pub async fn handle_inlay_hints( + &mut self, + file_path: String, + start_line: u32, + start_character: u32, + end_line: u32, + end_character: u32, + ) -> Result { + use crate::bridge::encoding::lsp_to_mcp_position; + + let path = PathBuf::from(&file_path); + let validated_path = self.validate_path(&path)?; + let client = self.get_client_for_file(&validated_path)?; + let uri = self + .document_tracker + .ensure_open(&validated_path, &client) + .await?; + + let lsp_start = mcp_to_lsp_position(start_line, start_character); + let lsp_end = mcp_to_lsp_position(end_line, end_character); + + let params = InlayHintParams { + text_document: TextDocumentIdentifier { uri }, + range: lsp_types::Range { + start: lsp_start, + end: lsp_end, + }, + work_done_progress_params: WorkDoneProgressParams::default(), + }; + + let timeout_duration = Duration::from_secs(30); + let response: Option> = client + .request("textDocument/inlayHint", params, timeout_duration) + .await?; + + let hints = response + .unwrap_or_default() + .into_iter() + .map(|hint| { + let (mcp_line, mcp_character) = lsp_to_mcp_position(hint.position); + let label = match hint.label { + InlayHintLabel::String(s) => s, + InlayHintLabel::LabelParts(parts) => parts + .into_iter() + .map(|p| p.value) + .collect::>() + .concat(), + }; + let tooltip = hint.tooltip.map(|t| match t { + lsp_types::InlayHintTooltip::String(s) => s, + lsp_types::InlayHintTooltip::MarkupContent(m) => m.value, + }); + InlayHintEntry { + position: Position2D { + line: mcp_line, + character: mcp_character, + }, + label, + kind: hint.kind.and_then(|k| { + serde_json::to_value(k) + .ok() + .and_then(|v| v.as_i64()) + .and_then(|n| u8::try_from(n).ok()) + }), + padding_left: hint.padding_left, + padding_right: hint.padding_right, + tooltip, + } + }) + .collect(); + + Ok(InlayHintsResult { hints }) + } } /// Extract hover contents as markdown string. +/// Convert LSP `Documentation` to a plain string. +fn extract_documentation(doc: lsp_types::Documentation) -> String { + match doc { + lsp_types::Documentation::String(s) => s, + lsp_types::Documentation::MarkupContent(m) => m.value, + } +} + +/// Normalize a `GotoDefinitionResponse` into a flat list of MCP `Location` values. +fn goto_response_to_locations( + response: Option, +) -> Vec { + let lsp_locs: Vec = match response { + Some(lsp_types::GotoDefinitionResponse::Scalar(loc)) => vec![loc], + Some(lsp_types::GotoDefinitionResponse::Array(locs)) => locs, + Some(lsp_types::GotoDefinitionResponse::Link(links)) => links + .into_iter() + .map(|link| lsp_types::Location { + uri: link.target_uri, + range: link.target_selection_range, + }) + .collect(), + None => vec![], + }; + + lsp_locs + .into_iter() + .map(|loc| Location { + uri: loc.uri.to_string(), + range: normalize_range(loc.range), + }) + .collect() +} + fn extract_hover_contents(contents: HoverContents) -> String { match contents { HoverContents::Scalar(marked_string) => marked_string_to_string(marked_string), diff --git a/crates/mcpls-core/src/lib.rs b/crates/mcpls-core/src/lib.rs index dfd246d..71d0d61 100644 --- a/crates/mcpls-core/src/lib.rs +++ b/crates/mcpls-core/src/lib.rs @@ -36,13 +36,126 @@ pub mod mcp; use std::path::PathBuf; use std::sync::Arc; -use bridge::Translator; +use bridge::resources::make_uri; +use bridge::{ResourceSubscriptions, Translator}; pub use config::ServerConfig; pub use error::Error; -use lsp::{LspServer, ServerInitConfig}; +use lsp::{LspNotification, LspServer, ServerInitConfig}; use rmcp::ServiceExt; -use tokio::sync::Mutex; -use tracing::{error, info, warn}; +use rmcp::model::ResourceUpdatedNotificationParam; +use tokio::sync::{Mutex, OnceCell}; +use tokio::task::JoinSet; +use tracing::{debug, error, info, warn}; + +/// Background task that drains LSP notifications, writes them to the cache, +/// and forwards `resources/updated` to the MCP peer when subscribed. +/// +/// The task operates in two phases without explicit state: +/// - **Phase A** (before peer is set): caches every notification, skips peer notify. +/// - **Phase B** (after peer is set): additionally fires `notify_resource_updated` +/// for subscribed `PublishDiagnostics` URIs. +/// +/// The task exits when: +/// - The LSP notification channel closes (`rx.recv()` returns `None`). +/// - The cancellation watch fires (or the sender is dropped). +/// - `notify_resource_updated` returns an error (peer disconnect / transport closed). +/// +/// # Note on lock contention (TODO critic-S4) +/// All cache writes acquire `Arc>`, which is the same lock used +/// by every MCP tool call. Splitting `NotificationCache` into its own `Arc` +/// would eliminate this contention. Tracked as a P2 follow-up. +pub(crate) async fn diagnostics_pump( + _lang: String, + mut rx: tokio::sync::mpsc::Receiver, + translator: Arc>, + subs: Arc, + peer_cell: Arc>>, + mut cancel_rx: tokio::sync::watch::Receiver, +) { + loop { + tokio::select! { + // Exit when cancellation is requested or the sender is dropped. + result = cancel_rx.changed() => { + // Err means the sender was dropped; treat as cancellation. + if result.is_err() || *cancel_rx.borrow() { + break; + } + } + msg = rx.recv() => { + let Some(notif) = msg else { break }; + match notif { + LspNotification::PublishDiagnostics(p) => { + // Always cache unconditionally. + { + let mut t = translator.lock().await; + t.notification_cache_mut() + .store_diagnostics(&p.uri, p.version, p.diagnostics); + } + + // Fast path: skip URI construction when nothing is subscribed. + if subs.is_empty().await { + continue; + } + + // Notify only when peer is ready and URI is subscribed. + let Some(peer) = peer_cell.get() else { continue }; + let Some(path) = bridge::uri_to_path(&p.uri) else { continue }; + let Ok(mcp_uri) = make_uri(&path) else { continue }; + + // TODO(critic-S3): on subscribe, replay cached diagnostics once + // so clients that subscribe after the first PublishDiagnostics + // do not have to wait for the next LSP push. + if !subs.contains(&mcp_uri).await { + continue; + } + + if peer + .notify_resource_updated(ResourceUpdatedNotificationParam::new( + mcp_uri, + )) + .await + .is_err() + { + // Peer disconnected; stop the pump. + break; + } + } + LspNotification::LogMessage(m) => { + let mut t = translator.lock().await; + t.notification_cache_mut() + .store_log(m.typ.into(), m.message); + } + LspNotification::ShowMessage(m) => { + let mut t = translator.lock().await; + t.notification_cache_mut() + .store_message(m.typ.into(), m.message); + } + LspNotification::Progress { .. } | LspNotification::Other { .. } => {} + } + } + } + } +} + +/// Register initialized LSP servers with the translator and extract notification receivers. +/// +/// Takes ownership of the `ServerInitResult`, extracts `notification_rx` from each server +/// before registration, and returns a map of language-id to receiver for the pump tasks. +fn register_servers( + mut result: lsp::ServerInitResult, + translator: &mut bridge::Translator, +) -> std::collections::HashMap> { + let mut receivers = std::collections::HashMap::new(); + for (lang, server) in &mut result.servers { + receivers.insert(lang.clone(), server.take_notification_rx()); + } + for (language_id, server) in result.servers { + let client = server.client().clone(); + translator.register_client(language_id.clone(), client); + translator.register_server(language_id.clone(), server); + } + receivers +} /// Resolve workspace roots from config or current directory. /// @@ -147,13 +260,16 @@ pub async fn serve(config: ServerConfig) -> Result<(), Error> { applicable_configs.len() ); + // notification_receivers collects per-language mpsc receivers used by the pump tasks. + let mut notification_receivers = std::collections::HashMap::new(); + if applicable_configs.is_empty() { warn!("No applicable LSP servers configured — starting in protocol-only mode"); } else { - // Spawn all servers with graceful degradation + // Spawn all servers with graceful degradation. let result = LspServer::spawn_batch(&applicable_configs).await; - // Handle the three possible outcomes + // Handle the three possible outcomes. if result.all_failed() { return Err(Error::AllServersFailedToInit { count: result.failure_count(), @@ -172,21 +288,36 @@ pub async fn serve(config: ServerConfig) -> Result<(), Error> { } } - // Register all successfully initialized servers + // Register servers and extract their notification receivers. let server_count = result.server_count(); - for (language_id, server) in result.servers { - let client = server.client().clone(); - translator.register_client(language_id.clone(), client); - translator.register_server(language_id.clone(), server); - } - + notification_receivers = register_servers(result, &mut translator); info!("Proceeding with {} LSP server(s)", server_count); } let translator = Arc::new(Mutex::new(translator)); + let subscriptions = Arc::new(ResourceSubscriptions::new()); + // Peer cell is populated after the MCP transport is established (Phase B). + let peer_cell = Arc::new(OnceCell::new()); + + // Cancellation for pump tasks: send `true` to request shutdown. + let (cancel_tx, cancel_rx) = tokio::sync::watch::channel(false); + let mut pumps: JoinSet<()> = JoinSet::new(); + + // Phase A: start pump tasks before MCP serve so no notifications are dropped + // while the transport is being established. + for (lang, rx) in notification_receivers { + pumps.spawn(diagnostics_pump( + lang, + rx, + Arc::clone(&translator), + Arc::clone(&subscriptions), + Arc::clone(&peer_cell), + cancel_rx.clone(), + )); + } info!("Starting MCP server with rmcp..."); - let mcp_server = mcp::McplsServer::new(translator); + let mcp_server = mcp::McplsServer::new(Arc::clone(&translator), Arc::clone(&subscriptions)); info!("MCPLS server initialized successfully"); info!("Listening for MCP requests on stdio..."); @@ -196,13 +327,24 @@ pub async fn serve(config: ServerConfig) -> Result<(), Error> { .await .map_err(|e| Error::McpServer(format!("Failed to start MCP server: {e}")))?; - service + // Phase B: peer is now available; pump tasks will start sending notifications. + if let Err(e) = peer_cell.set(service.peer().clone()) { + // The cell can only be set once; a second set means a logic error. + debug!("Peer cell already set ({}), ignoring", e); + } + + let result = service .waiting() .await - .map_err(|e| Error::McpServer(format!("MCP server error: {e}")))?; + .map(|_| ()) + .map_err(|e| Error::McpServer(format!("MCP server error: {e}"))); + + // Signal pump tasks to exit and wait for them. + let _ = cancel_tx.send(true); + while pumps.join_next().await.is_some() {} info!("MCPLS server shutting down"); - Ok(()) + result } #[cfg(test)] @@ -527,4 +669,136 @@ mod tests { } } } + + // ------------------------------------------------------------------ + // diagnostics_pump unit tests + // ------------------------------------------------------------------ + + #[allow(clippy::unwrap_used, clippy::expect_used)] + mod pump_tests { + use lsp_types::{PublishDiagnosticsParams, Uri}; + use tokio::sync::{mpsc, watch}; + + use super::*; + + fn make_translator() -> Arc> { + Arc::new(Mutex::new(Translator::new())) + } + + fn make_subs() -> Arc { + Arc::new(ResourceSubscriptions::new()) + } + + type PeerCell = Arc>>; + + fn make_peer_cell() -> PeerCell { + Arc::new(OnceCell::new()) + } + + /// `PublishDiagnostics` is cached even when the peer is not yet connected. + #[tokio::test] + async fn test_pump_caches_before_peer_set() { + let translator = make_translator(); + let subs = make_subs(); + let peer_cell = make_peer_cell(); + let (tx, rx) = mpsc::channel(8); + // Keep _cancel_tx alive: dropping it causes cancel_rx.changed() to return Err, + // which makes the pump exit before processing any messages. + let (_cancel_tx, cancel_rx) = watch::channel(false); + + let t = Arc::clone(&translator); + tokio::spawn(diagnostics_pump( + "rust".to_string(), + rx, + t, + Arc::clone(&subs), + Arc::clone(&peer_cell), + cancel_rx, + )); + + let uri: Uri = "file:///test/main.rs".parse().unwrap(); + tx.send(LspNotification::PublishDiagnostics( + PublishDiagnosticsParams { + uri: uri.clone(), + diagnostics: vec![], + version: None, + }, + )) + .await + .unwrap(); + drop(tx); + + // Poll until the pump processes the message or we time out. + let cached = tokio::time::timeout(std::time::Duration::from_secs(5), async { + loop { + tokio::task::yield_now().await; + let found = { + let guard = translator.lock().await; + guard + .notification_cache() + .get_diagnostics(uri.as_str()) + .is_some() + }; + if found { + return true; + } + tokio::time::sleep(std::time::Duration::from_millis(5)).await; + } + }) + .await + .expect("pump did not cache diagnostics within 5 s"); + assert!(cached, "diagnostics should be cached before peer is set"); + } + + /// Pump exits cleanly when the cancel watch sends `true`. + #[tokio::test] + async fn test_pump_exits_on_cancel() { + let translator = make_translator(); + let subs = make_subs(); + let peer_cell = make_peer_cell(); + let (_tx, rx) = mpsc::channel::(8); + let (cancel_tx, cancel_rx) = watch::channel(false); + + let handle = tokio::spawn(diagnostics_pump( + "rust".to_string(), + rx, + translator, + subs, + peer_cell, + cancel_rx, + )); + + cancel_tx.send(true).unwrap(); + // Pump must finish within a short time after cancellation. + tokio::time::timeout(std::time::Duration::from_millis(200), handle) + .await + .expect("pump did not exit within timeout") + .unwrap(); + } + + /// Pump exits when the cancel sender is dropped (Err branch). + #[tokio::test] + async fn test_pump_exits_when_cancel_sender_dropped() { + let translator = make_translator(); + let subs = make_subs(); + let peer_cell = make_peer_cell(); + let (_tx, rx) = mpsc::channel::(8); + let (cancel_tx, cancel_rx) = watch::channel(false); + + let handle = tokio::spawn(diagnostics_pump( + "rust".to_string(), + rx, + translator, + subs, + peer_cell, + cancel_rx, + )); + + drop(cancel_tx); // triggers Err in cancel_rx.changed() + tokio::time::timeout(std::time::Duration::from_millis(200), handle) + .await + .expect("pump did not exit within timeout") + .unwrap(); + } + } } diff --git a/crates/mcpls-core/src/lsp/lifecycle.rs b/crates/mcpls-core/src/lsp/lifecycle.rs index a9636b3..596ea01 100644 --- a/crates/mcpls-core/src/lsp/lifecycle.rs +++ b/crates/mcpls-core/src/lsp/lifecycle.rs @@ -192,6 +192,16 @@ impl std::fmt::Debug for LspServer { } impl LspServer { + /// Take the notification receiver out of this server, replacing it with a dummy channel. + /// + /// Use this to extract the receiver for a background pump task before registering + /// the server with the translator. After this call, the server's `notification_rx` + /// will never receive messages. + pub fn take_notification_rx(&mut self) -> tokio::sync::mpsc::Receiver { + let (_, dummy) = tokio::sync::mpsc::channel(1); + std::mem::replace(&mut self.notification_rx, dummy) + } + /// Spawn and initialize LSP server. /// /// This performs the complete initialization sequence: diff --git a/crates/mcpls-core/src/mcp/handlers.rs b/crates/mcpls-core/src/mcp/handlers.rs index 106e58f..565ab7f 100644 --- a/crates/mcpls-core/src/mcp/handlers.rs +++ b/crates/mcpls-core/src/mcp/handlers.rs @@ -8,22 +8,31 @@ use std::sync::Arc; use tokio::sync::Mutex; -use crate::bridge::Translator; +use crate::bridge::{ResourceSubscriptions, Translator}; /// Shared context for all tool handlers. /// -/// This struct holds the translator that converts MCP tool calls -/// to LSP requests and is shared across all tool implementations. +/// Holds the translator and subscription state. The MCP peer handle is not +/// stored here because resource-update notifications are sent by the pump +/// tasks in `lib.rs`, which own their own `Arc>>`. pub struct HandlerContext { /// Translator for converting MCP calls to LSP requests. pub translator: Arc>, + /// Set of resource URIs the MCP client has subscribed to. + pub subscriptions: Arc, } impl HandlerContext { /// Create a new handler context. #[must_use] - pub const fn new(translator: Arc>) -> Self { - Self { translator } + pub const fn new( + translator: Arc>, + subscriptions: Arc, + ) -> Self { + Self { + translator, + subscriptions, + } } } @@ -34,9 +43,9 @@ mod tests { #[test] fn test_handler_context_creation() { - let translator = Translator::new(); - let context = HandlerContext::new(Arc::new(Mutex::new(translator))); - // Context should be created successfully + let translator = Arc::new(Mutex::new(Translator::new())); + let subscriptions = Arc::new(ResourceSubscriptions::new()); + let context = HandlerContext::new(translator, subscriptions); assert!(Arc::strong_count(&context.translator) == 1); } } diff --git a/crates/mcpls-core/src/mcp/server.rs b/crates/mcpls-core/src/mcp/server.rs index f3af260..51d2d65 100644 --- a/crates/mcpls-core/src/mcp/server.rs +++ b/crates/mcpls-core/src/mcp/server.rs @@ -6,18 +6,24 @@ use std::sync::Arc; use rmcp::handler::server::wrapper::Parameters; -use rmcp::model::{Implementation, ServerCapabilities, ServerInfo}; -use rmcp::{ErrorData as McpError, ServerHandler, tool, tool_handler, tool_router}; +use rmcp::model::{ + Implementation, ListResourcesResult, RawResource, ReadResourceRequestParams, + ReadResourceResult, ResourceContents, ServerCapabilities, ServerInfo, SubscribeRequestParams, + UnsubscribeRequestParams, +}; +use rmcp::{ErrorData as McpError, RoleServer, ServerHandler, tool, tool_handler, tool_router}; use tokio::sync::Mutex; use super::handlers::HandlerContext; use super::tools::{ CachedDiagnosticsParams, CallHierarchyCallsParams, CallHierarchyPrepareParams, CodeActionsParams, CompletionsParams, DefinitionParams, DiagnosticsParams, - DocumentSymbolsParams, FormatDocumentParams, HoverParams, ReferencesParams, RenameParams, - ServerLogsParams, ServerMessagesParams, WorkspaceSymbolParams, + DocumentSymbolsParams, FormatDocumentParams, GoToImplementationParams, + GoToTypeDefinitionParams, HoverParams, InlayHintsParams, ReferencesParams, RenameParams, + ServerLogsParams, ServerMessagesParams, SignatureHelpParams, WorkspaceSymbolParams, }; -use crate::bridge::Translator; +use crate::bridge::resources::{make_uri, parse_uri}; +use crate::bridge::{ResourceSubscriptions, Translator}; /// MCP server that exposes LSP capabilities as tools. #[derive(Clone)] @@ -27,10 +33,13 @@ pub struct McplsServer { #[tool_router] impl McplsServer { - /// Create a new MCP server with the given translator. + /// Create a new MCP server with the given translator and subscriptions. #[must_use] - pub fn new(translator: Arc>) -> Self { - let context = Arc::new(HandlerContext::new(translator)); + pub fn new( + translator: Arc>, + subscriptions: Arc, + ) -> Self { + let context = Arc::new(HandlerContext::new(translator, subscriptions)); Self { context } } @@ -418,17 +427,249 @@ impl McplsServer { Err(e) => Err(McpError::internal_error(e.to_string(), None)), } } + + /// Get signature help at a position. + #[tool( + description = "Signature help at position. Returns parameter info, active signature/parameter, and documentation while typing a call." + )] + async fn get_signature_help( + &self, + Parameters(SignatureHelpParams { + file_path, + line, + character, + }): Parameters, + ) -> Result { + let result = { + let mut translator = self.context.translator.lock().await; + translator + .handle_signature_help(file_path, line, character) + .await + }; + + match result { + Ok(value) => serde_json::to_string(&value) + .map_err(|e| McpError::internal_error(format!("Serialization error: {e}"), None)), + Err(e) => Err(McpError::internal_error(e.to_string(), None)), + } + } + + /// Go to implementation locations. + #[tool( + description = "Implementation locations of trait method or interface member at position." + )] + async fn go_to_implementation( + &self, + Parameters(GoToImplementationParams { + file_path, + line, + character, + }): Parameters, + ) -> Result { + let result = { + let mut translator = self.context.translator.lock().await; + translator + .handle_implementation(file_path, line, character) + .await + }; + + match result { + Ok(value) => serde_json::to_string(&value) + .map_err(|e| McpError::internal_error(format!("Serialization error: {e}"), None)), + Err(e) => Err(McpError::internal_error(e.to_string(), None)), + } + } + + /// Go to type definition location. + #[tool( + description = "Type definition location of expression at position. Distinct from go-to-definition for variable bindings." + )] + async fn go_to_type_definition( + &self, + Parameters(GoToTypeDefinitionParams { + file_path, + line, + character, + }): Parameters, + ) -> Result { + let result = { + let mut translator = self.context.translator.lock().await; + translator + .handle_type_definition(file_path, line, character) + .await + }; + + match result { + Ok(value) => serde_json::to_string(&value) + .map_err(|e| McpError::internal_error(format!("Serialization error: {e}"), None)), + Err(e) => Err(McpError::internal_error(e.to_string(), None)), + } + } + + /// Get inlay hints for a range. + #[tool( + description = "Inlay hints in range. Returns inferred type/parameter annotations the editor would render inline." + )] + async fn get_inlay_hints( + &self, + Parameters(InlayHintsParams { + file_path, + start_line, + start_character, + end_line, + end_character, + }): Parameters, + ) -> Result { + let result = { + let mut translator = self.context.translator.lock().await; + translator + .handle_inlay_hints( + file_path, + start_line, + start_character, + end_line, + end_character, + ) + .await + }; + + match result { + Ok(value) => serde_json::to_string(&value) + .map_err(|e| McpError::internal_error(format!("Serialization error: {e}"), None)), + Err(e) => Err(McpError::internal_error(e.to_string(), None)), + } + } } #[tool_handler] impl ServerHandler for McplsServer { + async fn list_resources( + &self, + _request: Option, + _context: rmcp::service::RequestContext, + ) -> Result { + // TODO(critic-S5): paginate when max_documents == 0 (unlimited mode can produce + // very large single-page responses that may exceed transport buffers). + let resources: Vec<_> = { + let translator = self.context.translator.lock().await; + translator + .document_tracker() + .open_paths() + .filter_map(|path| { + let uri = make_uri(path) + .inspect_err(|e| { + tracing::warn!( + "Skipping path in list_resources (make_uri failed): {}: {e}", + path.display() + ); + }) + .ok()?; + let name = path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("unknown") + .to_string(); + let raw = RawResource::new(uri, name) + .with_mime_type("application/json") + .with_description("LSP diagnostics for this file"); + Some(rmcp::model::Annotated::new(raw, None)) + }) + .collect() + }; + + Ok(ListResourcesResult::with_all_items(resources)) + } + + async fn read_resource( + &self, + request: ReadResourceRequestParams, + _context: rmcp::service::RequestContext, + ) -> Result { + let path = + parse_uri(&request.uri).map_err(|e| McpError::invalid_params(e.to_string(), None))?; + + // Enforce workspace-root containment — mirrors the guard in every LSP tool. + { + let translator = self.context.translator.lock().await; + translator + .validate_path(&path) + .map_err(|e| McpError::invalid_params(e.to_string(), None))?; + } + + let lsp_uri = crate::bridge::path_to_uri(&path); + + // TODO(critic-S2): distinguish "file not tracked" from "file tracked but clean" + // in the response shape. Currently both return `{"diagnostics":null}` which is + // ambiguous for clients that need to know whether analysis has run yet. + let diagnostics = { + let translator = self.context.translator.lock().await; + translator + .notification_cache() + .get_diagnostics(lsp_uri.as_str()) + .cloned() + }; + + let json = serde_json::to_string(&diagnostics) + .map_err(|e| McpError::internal_error(format!("Serialization error: {e}"), None))?; + + Ok(ReadResourceResult::new(vec![ResourceContents::text( + json, + request.uri, + )])) + } + + async fn subscribe( + &self, + request: SubscribeRequestParams, + _context: rmcp::service::RequestContext, + ) -> Result<(), McpError> { + let path = + parse_uri(&request.uri).map_err(|e| McpError::invalid_params(e.to_string(), None))?; + + // Enforce workspace-root containment (same invariant as every LSP tool). + { + let translator = self.context.translator.lock().await; + translator + .validate_path(&path) + .map_err(|e| McpError::invalid_params(e.to_string(), None))?; + } + + // TODO(S3): If diagnostics are already cached for this URI, emit a synthetic + // notify_resource_updated so clients subscribing after initial workspace indexing + // don't have to wait for the next LSP push. Requires peer access from HandlerContext. + // Track as follow-up issue. + self.context + .subscriptions + .subscribe(request.uri) + .await + .map_err(|e| McpError::invalid_params(e, None))?; + + Ok(()) + } + + async fn unsubscribe( + &self, + request: UnsubscribeRequestParams, + _context: rmcp::service::RequestContext, + ) -> Result<(), McpError> { + // Parse the URI for consistency with subscribe validation. + parse_uri(&request.uri).map_err(|e| McpError::invalid_params(e.to_string(), None))?; + self.context.subscriptions.unsubscribe(&request.uri).await; + Ok(()) + } + fn get_info(&self) -> ServerInfo { let mut implementation = Implementation::new("mcpls", env!("CARGO_PKG_VERSION")); implementation.title = Some("MCPLS - MCP to LSP Bridge".to_string()); implementation.description = Some(env!("CARGO_PKG_DESCRIPTION").to_string()); implementation.website_url = Some("https://github.com/bug-ops/mcpls".to_string()); - let mut server_info = ServerInfo::new(ServerCapabilities::builder().enable_tools().build()); + let capabilities = ServerCapabilities::builder() + .enable_tools() + .enable_resources() + .enable_resources_subscribe() + .build(); + let mut server_info = ServerInfo::new(capabilities); server_info.server_info = implementation; server_info.instructions = Some( concat!( @@ -450,8 +691,9 @@ mod tests { use super::*; fn create_test_server() -> McplsServer { - let translator = Translator::new(); - McplsServer::new(Arc::new(Mutex::new(translator))) + let translator = Arc::new(Mutex::new(Translator::new())); + let subscriptions = Arc::new(ResourceSubscriptions::new()); + McplsServer::new(translator, subscriptions) } #[tokio::test] @@ -831,4 +1073,137 @@ mod tests { let result = server.get_server_messages(params).await; assert!(result.is_ok()); } + + #[tokio::test] + async fn test_get_signature_help_tool_with_params() { + let server = create_test_server(); + let params = Parameters(SignatureHelpParams { + file_path: "/test/file.rs".to_string(), + line: 10, + character: 5, + }); + + let result = server.get_signature_help(params).await; + assert!(result.is_err()); + } + + #[tokio::test] + async fn test_go_to_implementation_tool_with_params() { + let server = create_test_server(); + let params = Parameters(GoToImplementationParams { + file_path: "/test/file.rs".to_string(), + line: 10, + character: 5, + }); + + let result = server.go_to_implementation(params).await; + assert!(result.is_err()); + } + + #[tokio::test] + async fn test_go_to_type_definition_tool_with_params() { + let server = create_test_server(); + let params = Parameters(GoToTypeDefinitionParams { + file_path: "/test/file.rs".to_string(), + line: 10, + character: 5, + }); + + let result = server.go_to_type_definition(params).await; + assert!(result.is_err()); + } + + #[tokio::test] + async fn test_get_inlay_hints_tool_with_params() { + let server = create_test_server(); + let params = Parameters(InlayHintsParams { + file_path: "/test/file.rs".to_string(), + start_line: 1, + start_character: 1, + end_line: 10, + end_character: 1, + }); + + let result = server.get_inlay_hints(params).await; + assert!(result.is_err()); + } + + // ------------------------------------------------------------------ + // Resource handler tests (logic-level, avoiding rmcp::service::RequestContext + // which requires a live Peer with private fields) + // ------------------------------------------------------------------ + + /// `list_resources` returns an empty vec for a fresh translator with no open documents. + #[tokio::test] + async fn test_list_resources_returns_empty_when_no_open_documents() { + let server = create_test_server(); + let empty = { + let translator = server.context.translator.lock().await; + translator.document_tracker().open_paths().count() == 0 + }; + assert!(empty); + } + + /// `parse_uri` rejects `file://` scheme — ensures `read_resource` would return an error. + #[test] + fn test_read_resource_rejects_file_scheme() { + let result = parse_uri("file:///some/file.rs"); + assert!(result.is_err()); + } + + /// `parse_uri` rejects `https://` scheme. + #[test] + fn test_subscribe_rejects_https_scheme() { + let result = parse_uri("https://evil.com/file.rs"); + assert!(result.is_err()); + } + + /// `validate_path` rejects a non-existent path (canonicalize fails). + #[tokio::test] + async fn test_validate_path_rejects_nonexistent_path() { + use std::path::Path; + + let translator = Arc::new(Mutex::new(Translator::new())); + let result = { + let t = translator.lock().await; + t.validate_path(Path::new("/this/path/does/not/exist/at/all.rs")) + }; + assert!(result.is_err()); + } + + /// subscribe cap enforced: after `MAX_SUBSCRIPTIONS` entries, the next call returns `Err`. + #[tokio::test] + async fn test_subscription_cap_enforced_in_handler_context() { + use crate::bridge::resources::MAX_SUBSCRIPTIONS; + + let subscriptions = Arc::new(ResourceSubscriptions::new()); + for i in 0..MAX_SUBSCRIPTIONS { + subscriptions + .subscribe(format!("lsp-diagnostics:///file{i}.rs")) + .await + .unwrap(); + } + let over = subscriptions + .subscribe("lsp-diagnostics:///overflow.rs".to_string()) + .await; + assert!(over.is_err()); + } + + /// unsubscribing a URI that was never subscribed is a no-op (returns `false`, not an error). + #[tokio::test] + async fn test_unsubscribe_nonexistent_is_noop() { + let subscriptions = Arc::new(ResourceSubscriptions::new()); + let removed = subscriptions + .unsubscribe("lsp-diagnostics:///nonexistent.rs") + .await; + assert!(!removed); + } + + /// Server capabilities advertise resources support. + #[tokio::test] + async fn test_server_capabilities_include_resources() { + let server = create_test_server(); + let info = server.get_info(); + assert!(info.capabilities.resources.is_some()); + } } diff --git a/crates/mcpls-core/src/mcp/tools.rs b/crates/mcpls-core/src/mcp/tools.rs index f7c216b..95ecc92 100644 --- a/crates/mcpls-core/src/mcp/tools.rs +++ b/crates/mcpls-core/src/mcp/tools.rs @@ -249,3 +249,69 @@ pub struct ServerMessagesParams { const fn default_message_limit() -> usize { 20 } + +/// Parameters for the `get_signature_help` tool. +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[schemars(description = "Parameters for getting signature help at a position in a file.")] +pub struct SignatureHelpParams { + /// Absolute path to the file. + #[schemars(description = "Absolute path to the file.")] + pub file_path: String, + /// Line number (1-based). + #[schemars(description = "Line number (1-based).")] + pub line: u32, + /// Character/column number (1-based). + #[schemars(description = "Character/column number (1-based).")] + pub character: u32, +} + +/// Parameters for the `go_to_implementation` tool. +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[schemars(description = "Parameters for navigating to implementations of a symbol.")] +pub struct GoToImplementationParams { + /// Absolute path to the file. + #[schemars(description = "Absolute path to the file.")] + pub file_path: String, + /// Line number (1-based). + #[schemars(description = "Line number (1-based).")] + pub line: u32, + /// Character/column number (1-based). + #[schemars(description = "Character/column number (1-based).")] + pub character: u32, +} + +/// Parameters for the `go_to_type_definition` tool. +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[schemars(description = "Parameters for navigating to the type definition of an expression.")] +pub struct GoToTypeDefinitionParams { + /// Absolute path to the file. + #[schemars(description = "Absolute path to the file.")] + pub file_path: String, + /// Line number (1-based). + #[schemars(description = "Line number (1-based).")] + pub line: u32, + /// Character/column number (1-based). + #[schemars(description = "Character/column number (1-based).")] + pub character: u32, +} + +/// Parameters for the `get_inlay_hints` tool. +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[schemars(description = "Parameters for getting inlay hints in a range.")] +pub struct InlayHintsParams { + /// Absolute path to the file. + #[schemars(description = "Absolute path to the file.")] + pub file_path: String, + /// Start line (1-based). + #[schemars(description = "Start line (1-based).")] + pub start_line: u32, + /// Start character (1-based). + #[schemars(description = "Start character (1-based).")] + pub start_character: u32, + /// End line (1-based). + #[schemars(description = "End line (1-based).")] + pub end_line: u32, + /// End character (1-based). + #[schemars(description = "End character (1-based).")] + pub end_character: u32, +} diff --git a/crates/mcpls-core/tests/e2e/protocol_tests.rs b/crates/mcpls-core/tests/e2e/protocol_tests.rs index 9ef8814..5330ce7 100644 --- a/crates/mcpls-core/tests/e2e/protocol_tests.rs +++ b/crates/mcpls-core/tests/e2e/protocol_tests.rs @@ -64,7 +64,7 @@ fn test_e2e_list_tools() -> Result<()> { .as_array() .unwrap_or_else(|| panic!("tools should be an array")); - assert_eq!(tools.len(), 16, "Should have exactly 16 tools"); + assert_eq!(tools.len(), 20, "Should have exactly 20 tools"); let tool_names: Vec<&str> = tools.iter().filter_map(|t| t["name"].as_str()).collect(); @@ -85,6 +85,10 @@ fn test_e2e_list_tools() -> Result<()> { "get_cached_diagnostics", "get_server_logs", "get_server_messages", + "get_signature_help", + "go_to_implementation", + "go_to_type_definition", + "get_inlay_hints", ] { assert!(tool_names.contains(expected), "Should have {expected} tool"); }