🐛 fix(lsp): build valid file URIs on Windows using URIFromPath - #82
Open
LaelLuo wants to merge 1 commit into
Open
🐛 fix(lsp): build valid file URIs on Windows using URIFromPath#82LaelLuo wants to merge 1 commit into
LaelLuo wants to merge 1 commit into
Conversation
- Use protocol.URIFromPath for all file URIs (didOpen/didChange/didClose, RootURI, WorkspaceFolders) - Convert DocumentUri back to OS path via DocumentUri.Path() when closing files - Update tools (hover, rename_symbol, diagnostics, codelens) to use proper URIs - Fix watcher to emit proper file URIs Rationale: previously URIs like 'file://C:\\path' (with backslashes/two slashes) could be invalid for LSP servers like typescript-language-server, causing requests (e.g. workspace/symbol and opened files) to be ignored, leading to frequent 'not found' results on Windows
STRd6
pushed a commit
to STRd6/mcp-language-server
that referenced
this pull request
May 14, 2026
Replaces every "file://"+path and TrimPrefix(uri,"file://") with protocol.URIFromPath / DocumentUri.Path() across the LSP client, watcher, tools, and edit utilities. On Linux paths these produce identical strings, but Windows paths (with backslashes / drive letters) previously yielded URIs like file://C:\foo that some servers (notably typescript-language-server) silently rejected, which surfaced as "not found" responses on workspace/symbol and opened-file requests. Adapted from upstream PR isaacphi#82 (LaelLuo). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
STRd6
pushed a commit
to STRd6/mcp-language-server
that referenced
this pull request
May 14, 2026
Two bugs introduced when adapting upstream PR isaacphi#82 to use DocumentUri.Path() instead of strings.TrimPrefix("file://"): - pattern_interfaces.go now calls .Path() unconditionally on the BaseURI string, which panics if the LSP sends a value without the file:// scheme. Guard the call with a HasPrefix check and fall back to using the raw string as a path. - watcher.matchesPattern was calling .Path() a second time on the basePath returned by pattern_interfaces.GetBasePath(), which has already been converted to a filesystem path. The original TrimPrefix happened to be idempotent; .Path() is not, and panics on a plain path. Drop the redundant call. Surfaced by rust-analyzer integration tests, which are the only LSP in our matrix that emits a RelativePattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
STRd6
pushed a commit
to STRd6/mcp-language-server
that referenced
this pull request
May 14, 2026
Two CI failures on 54ec662, both reproducible only against gopls (the local test pass before push missed them because gopls was not installed): - tools.go:19 was an infinite recursive call in mcpServer.addTool. The bulk rename of s.mcpServer.AddTool -> s.addTool in 54ec662 rewrote the call inside the helper itself, so every tool invocation recursed into the gate. staticcheck SA5007 caught it; reverted that one call back to s.mcpServer.AddTool. - edit_file.go was constructing a WorkspaceEdit key via protocol.DocumentUri(filePath) -- raw cast, no scheme. The PR isaacphi#82 edit_file.go ApplyTextEdits adaptation in utilities/edit.go is now uri.Path() instead of TrimPrefix("file://"), and Path() panics on a non-file URI. Fix the construction site (use URIFromPath) and make utilities/edit.go defensive via a uriToPath helper that falls back to the raw string when the prefix is missing, so an analogous bug surfaces as a file-not-found error rather than a runtime panic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
judggernaut
added a commit
to judggernaut/mcp-language-server
that referenced
this pull request
Jul 3, 2026
Upstream PR isaacphi#82 (isaacphi/mcp-language-server), extended to cover several additional call sites the original PR missed. Every file:// URI in this codebase was built by naive string concatenation (fmt.Sprintf("file://%s", path) / "file://" + path) and parsed back out with strings.TrimPrefix(uri, "file://"). On Windows, a path like C:\x\y produces "file://C:\x\y", which is not a valid URI: it has backslashes instead of forward slashes, and only two slashes after the scheme instead of three. Some language servers (e.g. typescript-language-server) silently ignore didOpen/didChange notifications carrying invalid URIs, which surfaces as persistent 'not found' responses from every tool on Windows. protocol.URIFromPath / DocumentUri.Path() already existed in this codebase (internal/protocol/uri.go, ported from gopls) and correctly handle Windows drive letters, slash direction, and percent-encoding, but were only used in a few places. Replace every remaining manual 'file://' + path construction with URIFromPath, and every strings.TrimPrefix(uri, "file://") with DocumentUri.Path(), across: - internal/lsp/client.go: workspace folder/root URI, OpenFile, NotifyChange, SyncOpenFiles, CloseFile, IsFileOpen, CloseAllFiles - internal/tools/*.go: diagnostics, rename-symbol, hover, get/execute codelens, references, definition, lsp-utilities, utilities - internal/utilities/edit.go: ApplyTextEdits, ApplyDocumentChange (create/delete/rename file operations) - internal/protocol/pattern_interfaces.go: RelativePattern base path resolution, used by the workspace file watcher's glob matching - internal/watcher/watcher.go: file change event URIs While extending this fix, found and fixed a real, previously-latent bug: internal/tools/edit_file.go built its WorkspaceEdit key with protocol.DocumentUri(filePath) directly from a raw filesystem path, not a real URI. This 'worked' only by accident, since the old strings.TrimPrefix(uri, "file://") on the receiving end is a silent no-op on a string that never had that prefix, so the raw path round-tripped unchanged. Fixed to build a real URI with protocol.URIFromPath(filePath); caught immediately by the integration suite once DocumentUri.Path() started validating its input for real. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Fix frequent 'not found' in tools on Windows by constructing valid file URIs:
Rationale
Invalid URIs like 'file://C:\path' can be ignored by LSP servers (e.g., typescript-language-server), preventing indexing and causing 'not found' responses. This change normalizes URIs to 'file:///C:/path' and forward slashes, per LSP spec.
Build:
go build ./...passes.Tests: Root
go testpasses; integration tests depend on external language servers.