fix(lsp): improve C++/Java/Python language server support#2547
Open
yiliang114 wants to merge 2 commits intomainfrom
Open
fix(lsp): improve C++/Java/Python language server support#2547yiliang114 wants to merge 2 commits intomainfrom
yiliang114 wants to merge 2 commits intomainfrom
Conversation
Key changes: 1. Remove LspLanguageDetector — LSP is now fully config-driven via .lsp.json or extensions. No more auto-detected built-in presets that fail when the server binary is missing. 2. Add ensureDocumentOpen — send textDocument/didOpen before every document-level LSP request (definitions, references, hover, documentSymbol, implementations, prepareCallHierarchy, diagnostics, codeActions). This fixes the root cause of most methods returning empty results (Issue #2106, #1873). 3. Add retry mechanism for slow servers — when a freshly opened document yields empty results on non-TypeScript servers (jdtls, clangd, pylsp), wait 2s and retry once. This mirrors the existing retry logic in workspaceSymbols. 4. Handle LSP 3.17 WorkspaceSymbol format — location.range is now optional in normalizeSymbolResult, fixing jdtls workspace symbol responses that omit the range field. 5. Improve workspace symbol warmup — for non-TypeScript servers, open a workspace file before the first workspace/symbol request and retry on empty results after a warmup delay. 6. Track warmup-opened URIs — warmupTypescriptServer now returns the opened URI, which is registered with ensureDocumentOpen to prevent duplicate didOpen notifications. Closes #2106, closes #1873 Made-with: Cursor
yiliang114
requested review from
DennisYu07,
DragonnZhang,
LaZzyMan,
Mingholy,
gwinthis,
pomelo-nwu and
tanzhenxin
as code owners
Contributor
📋 Review SummaryThis PR fixes critical LSP document-level operations (definitions, references, hover, documentSymbol, etc.) that were returning empty results for non-TypeScript servers like jdtls (Java), clangd (C++), and pylsp (Python). The root cause was twofold: missing 🔍 General Feedback
🎯 Specific Feedback🔴 Critical
🟡 High
🟢 Medium
🔵 Low
✅ Highlights
|
…tests The CI was failing because fs.readdirSync returns Dirent<Buffer>[], not Dirent<string>[]. Updated all 8 type assertions to use the correct generic. Made-with: Cursor
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TLDR
Fix LSP document-level operations (definitions, references, hover, documentSymbol, etc.) returning empty results for non-TypeScript servers like jdtls (Java), clangd (C++), and pylsp (Python).
The root cause was twofold:
textDocument/didOpen— document-level requests were sent without first notifying the server that the file was open, causing servers like clangd to reject with error-32602: trying to get AST for non-added document.Screenshots / Video Demo
N/A — no user-facing UI change. The fix improves LSP tool responses in the agent.
Dive Deeper
Key changes:
Remove
LspLanguageDetector— LSP is now fully config-driven via.lsp.jsonor extensions. No more auto-detected built-in presets that fail when the server binary is missing.Add
ensureDocumentOpen— sendstextDocument/didOpenbefore every document-level LSP request. Tracks opened documents per server to avoid duplicate notifications. Reconnection clears the tracking so files are re-opened on the new connection.Add retry mechanism for slow servers — when a freshly opened document yields empty results on non-TypeScript servers, wait 2s (
DEFAULT_LSP_DOCUMENT_RETRY_DELAY_MS) and retry once. This mirrors the existing retry logic inworkspaceSymbols.Handle LSP 3.17 WorkspaceSymbol format —
location.rangeis now optional innormalizeSymbolResult, fixing jdtls workspace symbol responses that omit the range field.Improve workspace symbol warmup — for non-TypeScript servers, open a workspace file before the first
workspace/symbolrequest and retry on empty results after a warmup delay.Track warmup-opened URIs —
warmupTypescriptServernow returns the opened URI, registered withensureDocumentOpento prevent duplicatedidOpennotifications.Files changed (13):
NativeLspService.ts— core fix: ensureDocumentOpen + retry logicLspConfigLoader.ts— remove built-in presets, pure config-drivenLspLanguageDetector.ts— deleted (replaced by config-driven approach)LspResponseNormalizer.ts— optional range for LSP 3.17 WorkspaceSymbolLspServerManager.ts— warmupTypescriptServer returns URIconstants.ts— new delay constantsNativeLspService.test.ts— 14 tests (3 new for retry mechanism)LspConfigLoader.test.ts— 6 tests (4 new for config-driven behavior)lsp-e2e-test.ts— new E2E test covering all 12 LSP methodslsp.md— updated docs with C++/Java configuration examplesindex.ts— remove LspLanguageDetector exportshell-utils.ts— fix Buffer type in execCommandsessionService.test.ts— fix type assertionsReviewer Test Plan
.lsp.jsonwith a Java/C++/Python server:{ "java": { "command": "jdtls", "args": ["-data", "/tmp/jdtls-workspace"] }, "cpp": { "command": "clangd", "args": ["--background-index"] } }qwen --experimental-lsplsp documentSymbolon a Java/C++ file → should return symbolslsp hoveron a symbol → should return type infolsp goToDefinitionon a function call → should navigatelsp findReferenceson a symbol → should find all referenceslsp workspaceSymbolwith a query → should find matchesTesting Matrix
Linked issues / bugs
Closes #2106
Closes #1873
Related to #1514
Made with Cursor