Conversation
There was a problem hiding this comment.
Pull request overview
Fixes language detection when lsp_servers[].file_patterns are used by deriving/overlaying extension→language mappings from those patterns, ensuring files like **/*.c / **/*.h resolve to the configured server language (e.g., cpp) instead of falling back to defaults like c/plaintext.
Changes:
serve()now initializes theTranslatorwith an effective extension map built from both workspace mappings and LSP serverfile_patterns.- Added conservative glob-pattern extension extraction and
ServerConfig::build_effective_extension_map(). - Added unit tests covering file-pattern-derived overrides and ignoring complex patterns that don’t yield a simple extension.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/mcpls-core/src/lib.rs | Switches serve() to use the new effective extension map for translator initialization. |
| crates/mcpls-core/src/config/mod.rs | Adds pattern→extension inference, builds merged extension map on ServerConfig, and includes tests for the new behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2b424f9 to
155e79c
Compare
GitHub's default CodeQL setup only runs on PRs from the same repository. Fork PRs (like #72) are skipped because pull_request events from forks don't have write access to the base repo's security events. Add an explicit codeql.yml workflow using pull_request_target so CodeQL analysis runs for both fork and non-fork PRs. The workflow checks out the PR head SHA explicitly while keeping permissions minimal (security-events: write, contents: read only).
* fix(ci): add CodeQL workflow for fork pull requests GitHub's default CodeQL setup only runs on PRs from the same repository. Fork PRs (like #72) are skipped because pull_request events from forks don't have write access to the base repo's security events. Add an explicit codeql.yml workflow using pull_request_target so CodeQL analysis runs for both fork and non-fork PRs. The workflow checks out the PR head SHA explicitly while keeping permissions minimal (security-events: write, contents: read only). * fix(ci): remove unsafe checkout ref in pull_request_target workflow Checking out fork head SHA in a pull_request_target context grants untrusted code access to the privileged workflow environment. Remove the explicit ref override so checkout defaults to the base branch, which is the correct and secure behavior for CodeQL analysis. Fixes CodeQL alert: Checkout of untrusted code in trusted context * fix(ci): restore fork head checkout with security justification Restore ref: pull_request.head.sha for pull_request_target so CodeQL scans the actual fork PR changes instead of the base branch. This pattern is safe because the job has no access to secrets and only holds security-events: write + contents: read permissions. Added inline comment to suppress the CodeQL false-positive alert and document the security reasoning.
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
155e79c to
1353ef9
Compare
| /// Extract a file extension from a glob-like file pattern. | ||
| /// | ||
| /// Supports common patterns such as `**/*.rs` and `*.h`. | ||
| /// Returns `None` for patterns without a simple trailing extension. | ||
| fn extract_extension_from_pattern(pattern: &str) -> Option<String> { | ||
| let (_, ext) = pattern.rsplit_once('.')?; | ||
| if ext.is_empty() { | ||
| return None; | ||
| } | ||
|
|
||
| // Keep this conservative: only accept plain extension-like tokens. | ||
| if ext | ||
| .chars() | ||
| .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-') | ||
| { | ||
| Some(ext.to_string()) | ||
| } else { | ||
| None | ||
| } |
There was a problem hiding this comment.
Add direct unit tests for extract_extension_from_pattern
Currently this function is only tested indirectly through test_build_effective_extension_map_overrides_with_file_patterns and test_build_effective_extension_map_ignores_complex_patterns_without_extension.
Please add dedicated unit tests covering edge cases:
- Empty string pattern:
""→None - Pattern without dot:
"**/*"→None - Dotfile pattern:
".gitignore"→None - Multi-dot extensions:
"foo.tar.gz"→Some("gz")
This ensures the conservative parsing logic is well-validated.
Optional: Consider returning &str instead of String since rsplit_once already returns slices of the input.
bug-ops
left a comment
There was a problem hiding this comment.
Thanks for tackling this! A few items to polish before merge:
- test_serve_initializes_translator_with_extensions(): This test calls the old API (workspace.build_extension_map()). Update it to test the new API (config.build_effective_extension_map()) or clarify if it's testing workspace-level mappings in isolation.
- CHANGELOG.md: Document the default behavior change for C/C++ files.
Description
This change fixes language detection when files are matched via lsp_servers[].file_patterns (for example **/.h, **/.c). serve() now uses an effective extension map that merges workspace extension mappings with extensions inferred from each LSP server’s file_patterns, so C/C++ headers and sources can resolve to the configured server language instead of falling back to plaintext (or wrong defaults).
Type of Change
Related Issues
N/a
Checklist
Additional Notes