Conversation
Problem
While using `mcpls` in a C++ project I noticed that it always return
the following error when calling get references from a '.h' header file:
Error: tool call error: tool call failed for `mcpls/get_references`
Caused by:
tools/call failed: Mcp error: -32603: no LSP server configured for language: c
After reviewing the documentation and creating an mcpls.toml with the
following, it continued to not work
[[lsp_servers]]
language_id = "cpp"
command = "clangd"
args = ["--background-index", "--clang-tidy"]
file_patterns = ["**/*.cpp", "**/*.cc", "**/*.cxx", "**/*.hpp", "**/*.c", "**/*.h"]
# and/or with
[[lsp_servers]]
language_id = "c"
command = "clangd"
args = ["--background-index", "--clang-tidy"]
file_patterns = ["**/*.c", "**/*.h"]
With the above changing resulting in the language detection for matching
file patterns to "plaintext"
Error: tool call error: tool call failed for `mcpls/get_references`
Caused by:
tools/call failed: Mcp error: -32603: no LSP server configured for language: plaintext
What changed
- serve() now initializes the translator with an effective extension map
built from both workspace mappings and LSP server file patterns:
- crates/mcpls-core/src/lib.rs:114:114
- Added ServerConfig::build_effective_extension_map() to overlay
extensions inferred from file_patterns:
- crates/mcpls-core/src/config/mod.rs:287:287
- Added a small parser for simple glob extensions (e.g. **/*.h, *.c):
- crates/mcpls-core/src/config/mod.rs:123:123
Tests added
- Pattern-derived mapping overrides default extension mapping (.c/.h -> cpp):
- crates/mcpls-core/src/config/mod.rs:700:700
- Complex non-simple patterns are ignored safely:
- crates/mcpls-core/src/config/mod.rs:721:721
Verification
- New tests passed.
- Full mcpls-core unit/integration tests passed.
- Existing unrelated doctest failure remains in lsp/types.rs (pre-existing visibility issue).
Fix was implemented by Codex
Add direct unit coverage for extract_extension_from_pattern edge cases, including empty input, no-dot patterns, dotfiles, and multi-dot filenames. Tighten the parser to reject dotfile basenames so hidden files like .gitignore do not get treated as language extensions. Update the translator initialization test to assert against build_effective_extension_map(), which matches the runtime code path introduced by the earlier file_patterns fix. Document the resulting C/C++ language detection behavior change in CHANGELOG.md.
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.
github-actions
bot
added
documentation
|
Codex called this a "fix" though i think it might be more of new functionality. The basic utility is that using Clangd, get_diagnostics calls an unsupported method; this change redirects it to the get_cached_diagnostics silently. In my workflow and perhaps a better solution is to direct codex/claude to only use the get_cached_diagnostics in the first place; at the moment I implement a Usage reference for your mcpls in my AGENTS.md files over multiple projects (Rust, C, C++ and Go) and the agents tend to encounter language specific failures from time to time. |
There was a problem hiding this comment.
Pull request overview
This PR makes diagnostics retrieval more robust across LSP servers by using pull diagnostics when supported and falling back to cached publishDiagnostics when not, and it wires notification forwarding into normal server startup so diagnostics/logs/messages get cached during serve().
Changes:
- Add capability-aware diagnostics: prefer
textDocument/diagnostic, fall back to cachedtextDocument/publishDiagnostics(including on-32601). - Forward LSP notifications from clients into the translator cache during normal startup.
- Infer extension→language mappings from simple
lsp_servers[].file_patternsand overlay onto the workspace extension map, with tests and changelog entry.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/mcpls-core/src/mcp/server.rs | Updates MCP tool description for diagnostics to reflect fallback behavior. |
| crates/mcpls-core/src/lsp/mod.rs | Re-exports LspNotification for wider use. |
| crates/mcpls-core/src/lsp/lifecycle.rs | Adds spawn APIs that optionally forward notifications; adds a lightweight test constructor. |
| crates/mcpls-core/src/lib.rs | Hooks notification forwarding into serve() and switches to the effective extension map. |
| crates/mcpls-core/src/config/mod.rs | Adds extension extraction from file patterns and builds an effective extension map with tests. |
| crates/mcpls-core/src/bridge/translator.rs | Caches forwarded notifications and adds pull-diagnostics fallback logic + tests. |
| CHANGELOG.md | Documents the changed behavior for C/C++ language detection from file_patterns. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn create_test_client(language_id: &str) -> LspClient { | ||
| let mock_stdin = Command::new("cat") | ||
| .stdin(Stdio::piped()) | ||
| .spawn() | ||
| .unwrap() | ||
| .stdin | ||
| .take() | ||
| .unwrap(); | ||
|
|
||
| let mock_stdout = Command::new("tail") | ||
| .arg("-f") | ||
| .arg("/dev/null") | ||
| .stdout(Stdio::piped()) | ||
| .spawn() | ||
| .unwrap() | ||
| .stdout | ||
| .take() | ||
| .unwrap(); |
There was a problem hiding this comment.
create_test_client spawns helper subprocesses (cat, tail -f /dev/null) but drops the Child handles immediately and does not set kill_on_drop(true). This can leak long-lived processes (especially tail -f) for the duration of the test run and is also Unix-specific due to /dev/null. Consider using a single long-lived child with both stdin/stdout piped (as done in LspServer::new_for_tests) and keep the Child owned by a small test helper so it’s reliably terminated on drop (or add a dedicated in-memory test transport).
| let supports_pull_diagnostics = self.supports_pull_diagnostics(&validated_path); | ||
| let _uri = self | ||
| .document_tracker | ||
| .ensure_open(&validated_path, &client) | ||
| .await?; |
There was a problem hiding this comment.
let _uri = ... is used later to build TextDocumentIdentifier, so the leading underscore is misleading (it normally signals an intentionally unused binding). Renaming this to uri would improve readability and avoid suggesting the value is unused.
Description
Fix get_diagnostics for language servers that do not support pull diagnostics, such
as clangd in this C++ setup.
The change makes diagnostics retrieval capability-aware: when a server supports
textDocument/diagnostic, mcpls uses it; otherwise it falls back to cached
textDocument/publishDiagnostics results instead of returning -32601 method not found.
It also wires LSP notification forwarding into normal server startup so published
diagnostics, logs, and messages are actually captured during serve().
Type of Change
change)
Related Issues
Fixes #(issue number)
Checklist
Additional Notes
unsupported
publishDiagnostics, window/logMessage, and window/showMessage are available during
a real session.