From e21fe12ff058932ef6f78b874d0669b2b6b7c7f9 Mon Sep 17 00:00:00 2001 From: Werdgames <243669902+vcheckk@users.noreply.github.com> Date: Fri, 12 Jun 2026 18:02:54 -0700 Subject: [PATCH 1/2] fix(bridge): release translator lock before diagnostics request --- crates/mcpls-core/src/bridge/translator.rs | 103 ++++++++++++++++++--- crates/mcpls-core/src/mcp/server.rs | 22 +++-- 2 files changed, 105 insertions(+), 20 deletions(-) diff --git a/crates/mcpls-core/src/bridge/translator.rs b/crates/mcpls-core/src/bridge/translator.rs index e0a6de7..9ef3fce 100644 --- a/crates/mcpls-core/src/bridge/translator.rs +++ b/crates/mcpls-core/src/bridge/translator.rs @@ -118,7 +118,7 @@ impl Default for Translator { #[derive(Debug, Serialize)] #[serde(rename_all = "camelCase")] -struct DiagnosticRequestParams { +pub(crate) struct DiagnosticRequestParams { text_document: TextDocumentIdentifier, #[serde(skip_serializing_if = "Option::is_none")] identifier: Option, @@ -130,7 +130,9 @@ struct DiagnosticRequestParams { partial_result_params: PartialResultParams, } -fn diagnostic_request_params(text_document: TextDocumentIdentifier) -> DiagnosticRequestParams { +pub(crate) fn diagnostic_request_params( + text_document: TextDocumentIdentifier, +) -> DiagnosticRequestParams { DiagnosticRequestParams { text_document, identifier: None, @@ -140,6 +142,14 @@ fn diagnostic_request_params(text_document: TextDocumentIdentifier) -> Diagnosti } } +/// State needed to issue a diagnostics request without holding the translator lock. +pub(crate) struct PreparedDiagnosticsRequest { + /// LSP client for the target file's language. + pub(crate) client: LspClient, + /// Serialized-safe LSP diagnostics request params. + pub(crate) params: DiagnosticRequestParams, +} + /// Position in a document (1-based for MCP). #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Position2D { @@ -766,6 +776,28 @@ impl Translator { /// /// Returns an error if the LSP request fails or the file cannot be opened. pub async fn handle_diagnostics(&mut self, file_path: String) -> Result { + let PreparedDiagnosticsRequest { client, params } = + self.prepare_diagnostics_request(file_path).await?; + + let timeout_duration = Duration::from_secs(30); + let response: lsp_types::DocumentDiagnosticReportResult = client + .request("textDocument/diagnostic", params, timeout_duration) + .await?; + + Ok(Self::diagnostics_result_from_lsp_response(response)) + } + + /// Prepare a diagnostics request while holding translator state only for + /// path validation, client lookup, and lazy document open. + /// + /// # Errors + /// + /// Returns an error if the path is invalid, no client is registered for the + /// file type, or the file cannot be opened. + pub(crate) async fn prepare_diagnostics_request( + &mut self, + file_path: String, + ) -> Result { let path = PathBuf::from(&file_path); let validated_path = self.validate_path(&path)?; let client = self.get_client_for_file(&validated_path)?; @@ -774,13 +806,17 @@ impl Translator { .ensure_open(&validated_path, &client) .await?; - let params = diagnostic_request_params(TextDocumentIdentifier { uri }); - - let timeout_duration = Duration::from_secs(30); - let response: lsp_types::DocumentDiagnosticReportResult = client - .request("textDocument/diagnostic", params, timeout_duration) - .await?; + Ok(PreparedDiagnosticsRequest { + client, + params: diagnostic_request_params(TextDocumentIdentifier { uri }), + }) + } + /// Convert an LSP diagnostics response to mcpls' stable MCP response shape. + #[must_use] + pub(crate) fn diagnostics_result_from_lsp_response( + response: lsp_types::DocumentDiagnosticReportResult, + ) -> DiagnosticsResult { let diagnostics = match response { lsp_types::DocumentDiagnosticReportResult::Report(report) => match report { lsp_types::DocumentDiagnosticReport::Full(full) => { @@ -791,7 +827,7 @@ impl Translator { lsp_types::DocumentDiagnosticReportResult::Partial(_) => vec![], }; - let result = DiagnosticsResult { + DiagnosticsResult { diagnostics: diagnostics .into_iter() .map(|diag| Diagnostic { @@ -812,9 +848,7 @@ impl Translator { }), }) .collect(), - }; - - Ok(result) + } } /// Handle rename request. @@ -2103,6 +2137,51 @@ mod tests { assert!(value.get("previousResultId").is_none()); } + #[test] + fn test_diagnostics_result_from_lsp_response_preserves_mcp_shape() { + let response = lsp_types::DocumentDiagnosticReportResult::Report( + lsp_types::DocumentDiagnosticReport::Full( + lsp_types::RelatedFullDocumentDiagnosticReport { + related_documents: None, + full_document_diagnostic_report: lsp_types::FullDocumentDiagnosticReport { + result_id: Some("rust-analyzer".to_string()), + items: vec![lsp_types::Diagnostic { + range: lsp_types::Range { + start: lsp_types::Position { + line: 2, + character: 4, + }, + end: lsp_types::Position { + line: 2, + character: 9, + }, + }, + severity: Some(lsp_types::DiagnosticSeverity::ERROR), + code: Some(lsp_types::NumberOrString::String("E0308".to_string())), + source: Some("rust-analyzer".to_string()), + message: "mismatched types".to_string(), + related_information: None, + tags: None, + data: None, + code_description: None, + }], + }, + }, + ), + ); + + let result = Translator::diagnostics_result_from_lsp_response(response); + let value = serde_json::to_value(&result).unwrap(); + + assert!(value.get("diagnostics").is_some()); + assert!(value.get("items").is_none()); + assert_eq!(value["diagnostics"][0]["message"], "mismatched types"); + assert_eq!(value["diagnostics"][0]["code"], "E0308"); + assert_eq!(value["diagnostics"][0]["severity"], "error"); + assert_eq!(value["diagnostics"][0]["range"]["start"]["line"], 3); + assert_eq!(value["diagnostics"][0]["range"]["start"]["character"], 5); + } + #[test] fn test_validate_path_no_workspace_roots() { let translator = Translator::new(); diff --git a/crates/mcpls-core/src/mcp/server.rs b/crates/mcpls-core/src/mcp/server.rs index 51d2d65..e4c9122 100644 --- a/crates/mcpls-core/src/mcp/server.rs +++ b/crates/mcpls-core/src/mcp/server.rs @@ -128,16 +128,22 @@ impl McplsServer { &self, Parameters(DiagnosticsParams { file_path }): Parameters, ) -> Result { - let result = { + let prepared = { let mut translator = self.context.translator.lock().await; - translator.handle_diagnostics(file_path).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)), + translator.prepare_diagnostics_request(file_path).await } + .map_err(|e| McpError::internal_error(e.to_string(), None))?; + + let timeout_duration = tokio::time::Duration::from_secs(30); + let response: lsp_types::DocumentDiagnosticReportResult = prepared + .client + .request("textDocument/diagnostic", prepared.params, timeout_duration) + .await + .map_err(|e| McpError::internal_error(e.to_string(), None))?; + + let result = Translator::diagnostics_result_from_lsp_response(response); + serde_json::to_string(&result) + .map_err(|e| McpError::internal_error(format!("Serialization error: {e}"), None)) } /// Rename a symbol across the workspace. From c896d73c4af7de65d42962d097c1899ec2292144 Mon Sep 17 00:00:00 2001 From: Werdgames <243669902+vcheckk@users.noreply.github.com> Date: Fri, 12 Jun 2026 20:59:31 -0700 Subject: [PATCH 2/2] chore: keep diagnostics helpers crate-local --- crates/mcpls-core/src/bridge/translator.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/mcpls-core/src/bridge/translator.rs b/crates/mcpls-core/src/bridge/translator.rs index 9ef3fce..6e56388 100644 --- a/crates/mcpls-core/src/bridge/translator.rs +++ b/crates/mcpls-core/src/bridge/translator.rs @@ -116,6 +116,7 @@ impl Default for Translator { } } +#[allow(clippy::redundant_pub_crate)] #[derive(Debug, Serialize)] #[serde(rename_all = "camelCase")] pub(crate) struct DiagnosticRequestParams { @@ -130,6 +131,7 @@ pub(crate) struct DiagnosticRequestParams { partial_result_params: PartialResultParams, } +#[allow(clippy::redundant_pub_crate)] pub(crate) fn diagnostic_request_params( text_document: TextDocumentIdentifier, ) -> DiagnosticRequestParams { @@ -143,6 +145,7 @@ pub(crate) fn diagnostic_request_params( } /// State needed to issue a diagnostics request without holding the translator lock. +#[allow(clippy::redundant_pub_crate)] pub(crate) struct PreparedDiagnosticsRequest { /// LSP client for the target file's language. pub(crate) client: LspClient,