Skip to content

Commit edf64c6

Browse files
committed
fix(diagnostics): support published diagnostics fallback
Make get_diagnostics capability-aware so servers without pull diagnostics support fall back to cached published diagnostics instead of surfacing -32601. Wire LSP notification forwarding into normal server startup so publishDiagnostics, log messages, and showMessage notifications populate the translator cache during serve(). Add focused tests for notification ingestion and cached diagnostics fallback, and clarify the MCP tool description to reflect the new behavior.
1 parent 39057da commit edf64c6

5 files changed

Lines changed: 281 additions & 20 deletions

File tree

crates/mcpls-core/src/bridge/translator.rs

Lines changed: 203 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@ use tokio::time::Duration;
1818
use url::Url;
1919

2020
use super::state::{ResourceLimits, detect_language};
21-
use super::{DocumentTracker, NotificationCache};
21+
use super::{DocumentTracker, LogLevel, MessageType, NotificationCache};
2222
use crate::bridge::encoding::mcp_to_lsp_position;
2323
use crate::error::{Error, Result};
24-
use crate::lsp::{LspClient, LspServer};
24+
use crate::lsp::{LspClient, LspNotification, LspServer};
2525

2626
/// Translator handles MCP tool calls by converting them to LSP requests.
2727
#[derive(Debug)]
@@ -103,6 +103,28 @@ impl Translator {
103103
&mut self.notification_cache
104104
}
105105

106+
/// Store a forwarded LSP notification in the local cache.
107+
pub fn handle_notification(&mut self, notification: LspNotification) {
108+
match notification {
109+
LspNotification::PublishDiagnostics(params) => {
110+
self.notification_cache.store_diagnostics(
111+
&params.uri,
112+
params.version,
113+
params.diagnostics,
114+
);
115+
}
116+
LspNotification::LogMessage(params) => {
117+
self.notification_cache
118+
.store_log(LogLevel::from(params.typ), params.message);
119+
}
120+
LspNotification::ShowMessage(params) => {
121+
self.notification_cache
122+
.store_message(MessageType::from(params.typ), params.message);
123+
}
124+
LspNotification::Other { .. } => {}
125+
}
126+
}
127+
106128
// TODO: These methods will be implemented in Phase 3-5
107129
// Initialize and shutdown are now handled by LspServer in lifecycle.rs
108130

@@ -453,13 +475,27 @@ impl Translator {
453475

454476
/// Get a cloned LSP client for a file path based on language detection.
455477
fn get_client_for_file(&self, path: &Path) -> Result<LspClient> {
456-
let language_id = detect_language(path, &self.extension_map);
478+
let language_id = self.get_language_id_for_file(path);
457479
self.lsp_clients
458480
.get(&language_id)
459481
.cloned()
460482
.ok_or(Error::NoServerForLanguage(language_id))
461483
}
462484

485+
/// Resolve the language ID for a file path.
486+
fn get_language_id_for_file(&self, path: &Path) -> String {
487+
detect_language(path, &self.extension_map)
488+
}
489+
490+
/// Determine if the selected server for a file supports pull diagnostics.
491+
fn supports_pull_diagnostics(&self, path: &Path) -> bool {
492+
let language_id = self.get_language_id_for_file(path);
493+
self.lsp_servers
494+
.get(&language_id)
495+
.and_then(|server| server.capabilities().diagnostic_provider.as_ref())
496+
.is_some()
497+
}
498+
463499
/// Parse and validate a file URI, returning the validated path.
464500
///
465501
/// # Errors
@@ -668,23 +704,35 @@ impl Translator {
668704
let path = PathBuf::from(&file_path);
669705
let validated_path = self.validate_path(&path)?;
670706
let client = self.get_client_for_file(&validated_path)?;
671-
let uri = self
707+
let supports_pull_diagnostics = self.supports_pull_diagnostics(&validated_path);
708+
let _uri = self
672709
.document_tracker
673710
.ensure_open(&validated_path, &client)
674711
.await?;
675712

713+
if !supports_pull_diagnostics {
714+
return self.handle_cached_diagnostics(&file_path);
715+
}
716+
676717
let params = lsp_types::DocumentDiagnosticParams {
677-
text_document: TextDocumentIdentifier { uri },
718+
text_document: TextDocumentIdentifier { uri: _uri },
678719
identifier: None,
679720
previous_result_id: None,
680721
work_done_progress_params: WorkDoneProgressParams::default(),
681722
partial_result_params: PartialResultParams::default(),
682723
};
683724

684725
let timeout_duration = Duration::from_secs(30);
685-
let response: lsp_types::DocumentDiagnosticReportResult = client
726+
let response: lsp_types::DocumentDiagnosticReportResult = match client
686727
.request("textDocument/diagnostic", params, timeout_duration)
687-
.await?;
728+
.await
729+
{
730+
Ok(response) => response,
731+
Err(Error::LspServerError { code: -32601, .. }) => {
732+
return self.handle_cached_diagnostics(&file_path);
733+
}
734+
Err(error) => return Err(error),
735+
};
688736

689737
let diagnostics = match response {
690738
lsp_types::DocumentDiagnosticReportResult::Report(report) => match report {
@@ -1577,11 +1625,19 @@ fn convert_code_action(action: lsp_types::CodeAction) -> CodeAction {
15771625
#[allow(clippy::unwrap_used)]
15781626
mod tests {
15791627
use std::fs;
1628+
use std::process::Stdio;
15801629

1630+
use lsp_types::{
1631+
Diagnostic as LspDiagnostic, DiagnosticSeverity as LspDiagnosticSeverity, Position,
1632+
PositionEncodingKind, PublishDiagnosticsParams, Range as LspRange, ServerCapabilities, Uri,
1633+
};
15811634
use tempfile::TempDir;
1635+
use tokio::process::Command;
15821636
use url::Url;
15831637

15841638
use super::*;
1639+
use crate::config::LspServerConfig;
1640+
use crate::lsp::LspTransport;
15851641

15861642
#[test]
15871643
fn test_translator_new() {
@@ -2158,6 +2214,146 @@ mod tests {
21582214
assert_eq!(diags.diagnostics.len(), 0);
21592215
}
21602216

2217+
fn create_test_client(language_id: &str) -> LspClient {
2218+
let mock_stdin = Command::new("cat")
2219+
.stdin(Stdio::piped())
2220+
.spawn()
2221+
.unwrap()
2222+
.stdin
2223+
.take()
2224+
.unwrap();
2225+
2226+
let mock_stdout = Command::new("tail")
2227+
.arg("-f")
2228+
.arg("/dev/null")
2229+
.stdout(Stdio::piped())
2230+
.spawn()
2231+
.unwrap()
2232+
.stdout
2233+
.take()
2234+
.unwrap();
2235+
2236+
let transport = LspTransport::new(mock_stdin, mock_stdout);
2237+
let mut config = LspServerConfig::clangd();
2238+
config.language_id = language_id.to_string();
2239+
LspClient::from_transport(config, transport)
2240+
}
2241+
2242+
#[test]
2243+
fn test_handle_notification_stores_diagnostics() {
2244+
let mut translator = Translator::new();
2245+
let uri: Uri = "file:///workspace/test.cpp".parse().unwrap();
2246+
let diagnostic = LspDiagnostic {
2247+
range: LspRange {
2248+
start: Position {
2249+
line: 0,
2250+
character: 0,
2251+
},
2252+
end: Position {
2253+
line: 0,
2254+
character: 3,
2255+
},
2256+
},
2257+
severity: Some(LspDiagnosticSeverity::ERROR),
2258+
message: "test diagnostic".to_string(),
2259+
..Default::default()
2260+
};
2261+
2262+
translator.handle_notification(LspNotification::PublishDiagnostics(
2263+
PublishDiagnosticsParams {
2264+
uri: uri.clone(),
2265+
version: Some(1),
2266+
diagnostics: vec![diagnostic],
2267+
},
2268+
));
2269+
2270+
let cached = translator.notification_cache().get_diagnostics(uri.as_str()).unwrap();
2271+
assert_eq!(cached.diagnostics.len(), 1);
2272+
assert_eq!(cached.version, Some(1));
2273+
assert_eq!(cached.diagnostics[0].message, "test diagnostic");
2274+
}
2275+
2276+
#[test]
2277+
fn test_handle_notification_stores_logs_and_messages() {
2278+
let mut translator = Translator::new();
2279+
2280+
translator.handle_notification(LspNotification::LogMessage(lsp_types::LogMessageParams {
2281+
typ: lsp_types::MessageType::WARNING,
2282+
message: "log entry".to_string(),
2283+
}));
2284+
translator.handle_notification(LspNotification::ShowMessage(
2285+
lsp_types::ShowMessageParams {
2286+
typ: lsp_types::MessageType::INFO,
2287+
message: "user message".to_string(),
2288+
},
2289+
));
2290+
2291+
let logs = translator.notification_cache().get_logs();
2292+
assert_eq!(logs.len(), 1);
2293+
assert_eq!(logs[0].level, LogLevel::Warning);
2294+
assert_eq!(logs[0].message, "log entry");
2295+
2296+
let messages = translator.notification_cache().get_messages();
2297+
assert_eq!(messages.len(), 1);
2298+
assert_eq!(messages[0].message_type, MessageType::Info);
2299+
assert_eq!(messages[0].message, "user message");
2300+
}
2301+
2302+
#[tokio::test]
2303+
async fn test_handle_diagnostics_falls_back_to_cached_when_pull_unsupported() {
2304+
let temp_dir = TempDir::new().unwrap();
2305+
let test_file = temp_dir.path().join("test.cpp");
2306+
fs::write(&test_file, "int main() { return 0; }\n").unwrap();
2307+
2308+
let client = create_test_client("cpp");
2309+
let server = LspServer::new_for_tests(
2310+
client.clone(),
2311+
ServerCapabilities::default(),
2312+
PositionEncodingKind::UTF8,
2313+
);
2314+
2315+
let mut extension_map = HashMap::new();
2316+
extension_map.insert("cpp".to_string(), "cpp".to_string());
2317+
2318+
let mut translator = Translator::new().with_extensions(extension_map);
2319+
translator.set_workspace_roots(vec![temp_dir.path().to_path_buf()]);
2320+
translator.register_client("cpp".to_string(), client);
2321+
translator.register_server("cpp".to_string(), server);
2322+
2323+
let uri = Url::from_file_path(&test_file).unwrap();
2324+
let diagnostic = LspDiagnostic {
2325+
range: LspRange {
2326+
start: Position {
2327+
line: 0,
2328+
character: 4,
2329+
},
2330+
end: Position {
2331+
line: 0,
2332+
character: 8,
2333+
},
2334+
},
2335+
severity: Some(LspDiagnosticSeverity::WARNING),
2336+
message: "cached diagnostic".to_string(),
2337+
..Default::default()
2338+
};
2339+
let cached_uri: Uri = uri.as_str().parse().unwrap();
2340+
translator
2341+
.notification_cache_mut()
2342+
.store_diagnostics(&cached_uri, Some(1), vec![diagnostic]);
2343+
2344+
let result = translator
2345+
.handle_diagnostics(test_file.to_str().unwrap().to_string())
2346+
.await
2347+
.unwrap();
2348+
2349+
assert_eq!(result.diagnostics.len(), 1);
2350+
assert_eq!(result.diagnostics[0].message, "cached diagnostic");
2351+
assert!(matches!(
2352+
result.diagnostics[0].severity,
2353+
DiagnosticSeverity::Warning
2354+
));
2355+
}
2356+
21612357
#[test]
21622358
fn test_handle_server_logs_with_filter() {
21632359
use crate::bridge::notifications::LogLevel;

crates/mcpls-core/src/lib.rs

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,9 @@ use std::sync::Arc;
3939
use bridge::Translator;
4040
pub use config::ServerConfig;
4141
pub use error::Error;
42-
use lsp::{LspServer, ServerInitConfig};
42+
use lsp::{LspNotification, LspServer, ServerInitConfig};
4343
use rmcp::ServiceExt;
44-
use tokio::sync::Mutex;
44+
use tokio::sync::{Mutex, mpsc};
4545
use tracing::{error, info, warn};
4646

4747
/// Resolve workspace roots from config or current directory.
@@ -116,6 +116,16 @@ pub async fn serve(config: ServerConfig) -> Result<(), Error> {
116116

117117
let mut translator = Translator::new().with_extensions(extension_map);
118118
translator.set_workspace_roots(workspace_roots.clone());
119+
let translator = Arc::new(Mutex::new(translator));
120+
121+
let (notification_tx, mut notification_rx) = mpsc::channel::<LspNotification>(256);
122+
let notification_translator = Arc::clone(&translator);
123+
tokio::spawn(async move {
124+
while let Some(notification) = notification_rx.recv().await {
125+
let mut translator = notification_translator.lock().await;
126+
translator.handle_notification(notification);
127+
}
128+
});
119129

120130
// Build configurations for batch spawning with heuristics filtering
121131
let applicable_configs: Vec<ServerInitConfig> = config
@@ -148,7 +158,8 @@ pub async fn serve(config: ServerConfig) -> Result<(), Error> {
148158
);
149159

150160
// Spawn all servers with graceful degradation
151-
let result = LspServer::spawn_batch(&applicable_configs).await;
161+
let result =
162+
LspServer::spawn_batch_with_notifications(&applicable_configs, Some(notification_tx)).await;
152163

153164
// Handle the three possible outcomes
154165
if result.all_failed() {
@@ -178,16 +189,17 @@ pub async fn serve(config: ServerConfig) -> Result<(), Error> {
178189

179190
// Register all successfully initialized servers
180191
let server_count = result.server_count();
181-
for (language_id, server) in result.servers {
182-
let client = server.client().clone();
183-
translator.register_client(language_id.clone(), client);
184-
translator.register_server(language_id.clone(), server);
192+
{
193+
let mut translator = translator.lock().await;
194+
for (language_id, server) in result.servers {
195+
let client = server.client().clone();
196+
translator.register_client(language_id.clone(), client);
197+
translator.register_server(language_id.clone(), server);
198+
}
185199
}
186200

187201
info!("Proceeding with {} LSP server(s)", server_count);
188202

189-
let translator = Arc::new(Mutex::new(translator));
190-
191203
info!("Starting MCP server with rmcp...");
192204
let mcp_server = mcp::McplsServer::new(translator);
193205

0 commit comments

Comments
 (0)