fix(lsp): 修复 LSP 文档、isPathSafe 限制，并提升 LSP 工具调用率#3615
Merged
Conversation
- Remove reference to non-existent `packages/cli/LSP_DEBUGGING_GUIDE.md` - Remove reference to unimplemented `/lsp status` slash command - Replace incorrect `DEBUG=lsp*` env var with actual debug log location (`~/.qwen/debug/` session files with `[LSP]` tag) - Remove external Claude Code documentation links (`code.claude.com`) - Document `isPathSafe` constraint: absolute paths outside workspace are blocked, users must add server binary directory to PATH - Add practical troubleshooting: `ps aux | grep <server>` to check if the server process is actually running - Add clangd-specific guidance: `--background-index`, `compile_commands.json` location, and `--compile-commands-dir` usage - Simplify trust documentation (remove vague "configure in settings")
Previously, `isPathSafe` rejected any command containing a path separator that resolved outside the workspace directory. This blocked legitimate use cases where users specify absolute paths to language server binaries (e.g. `/usr/bin/clangd`, `/opt/tools/jdtls/bin/jdtls`). The fix allows: - Bare command names resolved via PATH (unchanged) - Absolute paths (explicit user intent, already gated by trust checks) - Relative paths within the workspace (unchanged) Only relative paths that traverse outside the workspace (e.g. `../../malicious-binary`) are still blocked. Closes: server silently fails to start when users configure absolute paths in `.lsp.json`, with only a debug log warning visible.
…abled
The model was not using the LSP tool because the system prompt's
"Tool Usage" section never mentioned it. The tool description alone
("ALWAYS use LSP as the PRIMARY tool") was insufficient — models
follow system prompt instructions more reliably than tool descriptions.
Changes:
- getCoreSystemPrompt() accepts `options.lspEnabled` parameter
- When LSP is enabled, injects an instruction in the Tool Usage section
telling the model to ALWAYS use the LSP tool FIRST for code
intelligence queries (definitions, references, hover, symbols, etc.)
instead of falling back to grep/readfile
- Updated client.ts to pass config.isLspEnabled() to the prompt builder
- Updated test mocks and snapshots
The model avoided calling LSP for findReferences, hover, etc. because
these operations required filePath + line + character which the user
rarely provides. The model would read files directly instead.
Changes:
- Add `symbolName` optional parameter to LspTool
- When symbolName is provided without line/character, auto-resolve
the symbol's position via workspaceSymbol before executing the
actual operation (findReferences, hover, goToImplementation, etc.)
- Update tool description with examples showing symbolName usage
- Move LSP priority instruction to top of system prompt for visibility
- Add debug logging for LSP prompt injection
This enables natural queries like:
{operation: "findReferences", symbolName: "Calculator"}
{operation: "hover", symbolName: "addShape"}
without requiring the user to know exact file positions.
When LSP is enabled, the model often chose grep or readfile instead of LSP for code intelligence queries. Now the competing tools' descriptions include a note reminding the model to use the LSP tool for definitions, references, symbols, hover, diagnostics, etc. This "push-pull" approach: - System prompt pushes toward LSP (top-level priority instruction) - Grep/ReadFile descriptions pull away from code intelligence usage
…supported The doc still said "absolute paths outside the workspace are not supported" but the code was changed to allow them. Updated all three places (Required Fields table, Troubleshooting, Debugging) to reflect that absolute paths are now accepted.
Collaborator
Author
LSP 功能 tmux 实测记录测试环境：Qwen Code v0.15.2 (worktree build), C++ Demo (cpp-demo) — clangd 19.1.71. workspaceSymbol 查找 Calculator 类 2. goToDefinition 跳转到头文件 3. hover 获取类型信息（via symbolName 自动解析） 4. documentSymbol 列出文件符号 5. hover 在 calculator.cpp 中 6. diagnostics TypeScript Demo (ts-demo) — typescript-language-serverJava Demo (java-demo) — jdtlsPython Demo (python-demo) — pylsp 1.14.0Debug 日志 |
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. |
Collaborator
Author
Verification update for LSP changesI narrowed the PR scope back down to LSP-related files only:
I also pushed one follow-up fix:
This fixes the Windows-only LSP unit test failure where formatted locations used Local verificationValidated against latest commit Commands run locally: npm run build
npm run bundle
cd packages/core && npx vitest run src/tools/lsp.test.tsResults: GitHub checks after the latest push, at the time of this report: Note: the previous Windows LSP failures were caused by path separator expectations in E2E smoke via tmuxAll smoke runs used the freshly bundled CLI with One non-blocking warning appears once per LSP tool execution: All four tmux sessions completed with Python
LSP input: {
"operation": "goToDefinition",
"symbolName": "SimpleCalculator"
}LSP output: Final result: TypeScript
LSP input: {
"operation": "goToDefinition",
"symbolName": "createCalculator"
}LSP output: Final result: Java
LSP inputs: {
"operation": "goToDefinition",
"symbolName": "computeSum"
}{
"operation": "hover",
"symbolName": "computeSum"
}{
"operation": "findReferences",
"symbolName": "computeSum"
}LSP outputs: Final result: C++
LSP input: {
"operation": "findReferences",
"symbolName": "Calculator"
}LSP output: Final result: ConclusionThe LSP tool is now triggered successfully in all four smoke demos: Python, TypeScript, Java, and C++. The results are useful for the requested code-intelligence tasks, and the PR no longer depends on broader prompt/grep/core tool changes to make LSP usable. |
yiliang114
force-pushed
the
worktree-fix-lsp-docs-and-bugs
branch
from
8372cd5 to
63bdf49
Compare
- Add test for intermediate path traversal (./a/../../../etc/passwd) - Add test for forward-slash relative paths (tools/clangd) - Replace Chinese JSDoc with English on requestUserConsent
This was referenced Apr 27, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes LSP configuration/documentation gaps and updates isPathSafe so .lsp.json can use absolute-path language server commands without being blocked, improving LSP server startup reliability.
Changes:
- Allow absolute command paths in
isPathSafewhile still blocking workspace-escaping relative paths. - Add unit tests covering
isPathSafescenarios (bare commands, absolute paths, safe/unsafe relative paths). - Update LSP user docs to reflect supported
commandformats and improve troubleshooting/debugging guidance.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/core/src/lsp/LspServerManager.ts | Loosens path-safety check to permit absolute command paths; updates related comments. |
| packages/core/src/lsp/LspServerManager.test.ts | Adds unit tests validating isPathSafe behavior across path forms. |
| docs/users/features/lsp.md | Updates .lsp.json documentation and troubleshooting/debug instructions. |
Comments suppressed due to low confidence (1)
packages/core/src/lsp/LspServerManager.ts:662
isPathSafeusesresolvedPath.startsWith(resolvedWorkspacePath + path.sep)which breaks whenworkspacePathis the filesystem root (e.g./):resolvedWorkspacePath + path.sepbecomes//, causing all non-rootresolvedPaths to be treated as unsafe. Consider reusing the existingisWithinRoot()helper (packages/core/src/utils/fileUtils.ts) or applying the same “rootWithSeparator” logic here to correctly handle root paths and trailing separators.
const resolvedWorkspacePath = path.resolve(workspacePath);
const basePath = cwd ? path.resolve(cwd) : resolvedWorkspacePath;
const resolvedPath = path.resolve(basePath, command);
return (
resolvedPath.startsWith(resolvedWorkspacePath + path.sep) ||
resolvedPath === resolvedWorkspacePath
);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
665
to
667
| /** | ||
| * 请求用户确认启动 LSP 服务器 | ||
| * Request user consent before starting an LSP server. | ||
| */ |
The method only checks workspace trust level and does not actually prompt the user for consent. Rename the method and update the JSDoc and call-site log message to accurately reflect the behavior.
xaelistic
pushed a commit
to xaelistic/qwen-code
that referenced
this pull request
Jun 7, 2026
* fix(docs): correct outdated and inaccurate LSP documentation
- Remove reference to non-existent `packages/cli/LSP_DEBUGGING_GUIDE.md`
- Remove reference to unimplemented `/lsp status` slash command
- Replace incorrect `DEBUG=lsp*` env var with actual debug log location
(`~/.qwen/debug/` session files with `[LSP]` tag)
- Remove external Claude Code documentation links (`code.claude.com`)
- Document `isPathSafe` constraint: absolute paths outside workspace
are blocked, users must add server binary directory to PATH
- Add practical troubleshooting: `ps aux | grep <server>` to check
if the server process is actually running
- Add clangd-specific guidance: `--background-index`, `compile_commands.json`
location, and `--compile-commands-dir` usage
- Simplify trust documentation (remove vague "configure in settings")
* fix(lsp): allow absolute paths in LSP server command configuration
Previously, `isPathSafe` rejected any command containing a path
separator that resolved outside the workspace directory. This blocked
legitimate use cases where users specify absolute paths to language
server binaries (e.g. `/usr/bin/clangd`, `/opt/tools/jdtls/bin/jdtls`).
The fix allows:
- Bare command names resolved via PATH (unchanged)
- Absolute paths (explicit user intent, already gated by trust checks)
- Relative paths within the workspace (unchanged)
Only relative paths that traverse outside the workspace (e.g.
`../../malicious-binary`) are still blocked.
Closes: server silently fails to start when users configure absolute
paths in `.lsp.json`, with only a debug log warning visible.
* feat(lsp): inject LSP priority instruction into system prompt when enabled
The model was not using the LSP tool because the system prompt's
"Tool Usage" section never mentioned it. The tool description alone
("ALWAYS use LSP as the PRIMARY tool") was insufficient — models
follow system prompt instructions more reliably than tool descriptions.
Changes:
- getCoreSystemPrompt() accepts `options.lspEnabled` parameter
- When LSP is enabled, injects an instruction in the Tool Usage section
telling the model to ALWAYS use the LSP tool FIRST for code
intelligence queries (definitions, references, hover, symbols, etc.)
instead of falling back to grep/readfile
- Updated client.ts to pass config.isLspEnabled() to the prompt builder
- Updated test mocks and snapshots
* feat(lsp): add symbolName parameter for position-free LSP queries
The model avoided calling LSP for findReferences, hover, etc. because
these operations required filePath + line + character which the user
rarely provides. The model would read files directly instead.
Changes:
- Add `symbolName` optional parameter to LspTool
- When symbolName is provided without line/character, auto-resolve
the symbol's position via workspaceSymbol before executing the
actual operation (findReferences, hover, goToImplementation, etc.)
- Update tool description with examples showing symbolName usage
- Move LSP priority instruction to top of system prompt for visibility
- Add debug logging for LSP prompt injection
This enables natural queries like:
{operation: "findReferences", symbolName: "Calculator"}
{operation: "hover", symbolName: "addShape"}
without requiring the user to know exact file positions.
* feat(lsp): add LSP reminder to grep/readfile tool descriptions
When LSP is enabled, the model often chose grep or readfile instead
of LSP for code intelligence queries. Now the competing tools'
descriptions include a note reminding the model to use the LSP tool
for definitions, references, symbols, hover, diagnostics, etc.
This "push-pull" approach:
- System prompt pushes toward LSP (top-level priority instruction)
- Grep/ReadFile descriptions pull away from code intelligence usage
* fix(docs): align LSP doc with isPathSafe change — absolute paths now supported
The doc still said "absolute paths outside the workspace are not
supported" but the code was changed to allow them. Updated all
three places (Required Fields table, Troubleshooting, Debugging)
to reflect that absolute paths are now accepted.
* fix(lsp): improve symbol-based tool resolution
* fix(lsp): normalize display paths across platforms
* fix(lsp): narrow docs and path safety changes
* fix(lsp): add edge-case tests for isPathSafe and fix Chinese comment
- Add test for intermediate path traversal (./a/../../../etc/passwd)
- Add test for forward-slash relative paths (tools/clangd)
- Replace Chinese JSDoc with English on requestUserConsent
* fix(lsp): rename requestUserConsent to checkWorkspaceTrust
The method only checks workspace trust level and does not actually
prompt the user for consent. Rename the method and update the JSDoc
and call-site log message to accurately reflect the behavior.
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
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.
Summary
修复 LSP 功能的文档和
isPathSafe安全检查问题，使.lsp.json中的绝对路径命令配置正常工作。变更内容
isPathSafe 修复 — 允许
.lsp.json中使用绝对路径的语言服务器命令：/usr/bin/clangd等 workspace 外的绝对路径被安全检查拦截，服务器静默启动失败文档修复 (
docs/users/features/lsp.md)：command字段说明，明确支持绝对路径packages/cli/LSP_DEBUGGING_GUIDE.md引用/lsp statusslash command 引用DEBUG=lsp*→~/.qwen/debug/session 日志）code.claude.com外部链接--background-index、compile_commands.json）Validation
Unit Tests
isPathSafe 测试覆盖
clangd)/usr/bin/clangd)./tools/clangd)../bin/evil)./tools/../../../etc/passwd)Scope / Risk
Testing Matrix