Repository navigation
Normalize document URIs during decoding - #64396
Open
Jake Bailey (jakebailey) wants to merge 4 commits into
Open
Jake Bailey (jakebailey) wants to merge 4 commits into
Jake Bailey (jakebailey) wants to merge 4 commits into
Conversation
Some clients send file URIs with empty or relative paths. Passing those spellings through can produce relative file names that reach the VFS. The reference client applies the same URI conversion to both LSP URI aliases. Normalize both at protocol decode time so document and workspace URIs consistently produce rooted file paths while custom schemes remain unchanged.
URI-to-filename and filename-to-URI conversion are protocol boundary operations. Keeping outbound formatting in lsconv separates it from the normalization and inbound conversion rules it must match. Keep both directions in lsproto while preserving existing call sites. Verify that normalization also applies when document URIs are decoded as JSON object keys.
The filename-to-URI conversion is owned by lsproto, but retaining a forwarding helper in lsconv leaves ownership ambiguous and keeps its tests in the wrong package. Call the protocol constructor directly and keep URI conversion tests beside the implementation.
typescript-automation
Bot
added
Author: Team
For Uncommitted Bug
Copilot started reviewing on behalf of
Jake Bailey (jakebailey)
September 22, 2026 22:11
View session
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The normalization addresses the reported crash and is covered by focused decoding, map-key, and server regression tests.
Review effort: Balanced
Findings: None
What changed in this PR
Normalizes malformed or relative document URIs during LSP decoding, preventing empty file:// paths from reaching project filesystem logic.
Changes:
- Normalize
DocumentUriandURIvalues during JSON decoding. - Move filename-to-URI conversion into
lsprotoand update callers. - Add unit and server regression coverage.
| File | Description |
|---|---|
tsc/internal/testutil/autoimporttestutil/fixtures.go |
Uses the relocated URI helper. |
tsc/internal/project/watch.go |
Updates watcher URI conversion. |
tsc/internal/project/untitled_test.go |
Updates untitled URI round-trip test. |
tsc/internal/project/snapshotfs.go |
Updates snapshot filesystem URI conversion. |
tsc/internal/project/session.go |
Updates diagnostic URI conversion. |
tsc/internal/project/projectreferencesprogram_test.go |
Migrates project-reference tests. |
tsc/internal/lsp/server.go |
Migrates server URI creation. |
tsc/internal/lsp/server_projectreference_updates_test.go |
Migrates project-update tests. |
tsc/internal/lsp/server_projectinfo_test.go |
Adds empty-path URI regression coverage. |
tsc/internal/lsp/server_completion_test.go |
Migrates completion tests. |
tsc/internal/lsp/replay_test.go |
Migrates replay setup. |
tsc/internal/lsp/lspwatcher/lspwatcher.go |
Migrates watcher event URIs. |
tsc/internal/lsp/lsproto/lsp.go |
Implements URI decoding normalization and conversion helper. |
tsc/internal/lsp/lsproto/lsp_uri_test.go |
Relocates URI conversion tests. |
tsc/internal/lsp/lsproto/lsp_json_test.go |
Tests decoding normalization and failures. |
tsc/internal/ls/source_map.go |
Migrates source-map locations. |
tsc/internal/ls/lsconv/converters.go |
Removes the old helper and updates conversions. |
tsc/internal/ls/lsconv/converters_test.go |
Removes relocated URI tests. |
tsc/internal/ls/inlay_hints.go |
Migrates inlay-hint locations. |
tsc/internal/ls/findallreferences.go |
Migrates reference locations. |
tsc/internal/ls/file_rename.go |
Migrates rename URIs. |
tsc/internal/ls/documenthighlights.go |
Migrates highlight URIs. |
tsc/internal/ls/definition.go |
Migrates definition links. |
tsc/internal/ls/codeactions.go |
Migrates code-action URIs. |
tsc/internal/ls/callhierarchy.go |
Migrates call-hierarchy URIs. |
tsc/internal/ls/autoimport/registry.go |
Migrates registry change URIs. |
tsc/internal/ls/autoimport/registry_test.go |
Migrates auto-import tests. |
tsc/internal/fourslash/tests/getEditsForFileRenameWithSolutionConfigFile_test.go |
Migrates rename test URIs. |
tsc/internal/fourslash/test_parser.go |
Migrates marker locations. |
tsc/internal/fourslash/statebaseline.go |
Migrates baseline request URIs. |
tsc/internal/fourslash/semantictokens.go |
Migrates semantic-token requests. |
tsc/internal/fourslash/fourslash.go |
Migrates fourslash URI creation. |
tsc/internal/fourslash/baselineutil.go |
Migrates baseline grouping URIs. |
tsc/internal/api/requestfilesystem/filechanges.go |
Migrates API filesystem change URIs. |
tsc/internal/api/proto.go |
Migrates API document identifiers. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
# Conflicts: # tsc/internal/api/proto.go # tsc/internal/api/requestfilesystem/filechanges.go # tsc/internal/fourslash/baselineutil.go # tsc/internal/fourslash/fourslash.go # tsc/internal/ls/autoimport/registry.go # tsc/internal/ls/autoimport/registry_test.go # tsc/internal/ls/lsconv/converters.go # tsc/internal/ls/lsconv/converters_test.go # tsc/internal/lsp/lspwatcher/lspwatcher.go # tsc/internal/lsp/replay_test.go # tsc/internal/lsp/server.go # tsc/internal/project/projectreferencesprogram_test.go # tsc/internal/project/session.go # tsc/internal/project/snapshotfs.go # tsc/internal/project/watch.go # tsc/internal/testutil/autoimporttestutil/fixtures.go
Yuki Iwanaga (creasty)
added a commit
to creasty/dotfiles
that referenced
this pull request
Oct 12, 2026
TypeScript 7's native port moved from microsoft/typescript-go, now archived, into microsoft/TypeScript, where the crash is microsoft/TypeScript#64300 and microsoft/TypeScript#64396 would fix it: CLAUDE.md now names the server by what it is and links both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Yuki Iwanaga (creasty)
added a commit
to creasty/dotfiles
that referenced
this pull request
Oct 12, 2026
…et once a server stops (#217) * Keep pauses quiet once the server that highlights has stopped Resting the cursor highlights the symbol under it and its references. The autocmds that ask for them were set up for each buffer as a server that can highlight attached to it, and stayed when that server went away (crashed or stopped). If another server was still attached to the buffer (codebook, tailwindcss), every pause in it then asked again, and Neovim answered with an error and a Press ENTER prompt: 'vim.lsp: method "textDocument/documentHighlight" is not supported by any server activated for this buffer'. Since the autocmds came in with Neovim's LSP client (#113, 2026-09-27); seen when tsc crashed on a buffer without a name, in every TypeScript buffer of the project. One CursorHold autocmd for all buffers now asks only while an attached server supports it, so nothing outlives the server. get_clients() lists a client for a buffer only once it is initialized and attached, and Neovim sends didOpen in the same call that makes both true, so a pause before LspAttach asks no server that hasn't opened the document. Clearing the highlights as the cursor moves runs in every buffer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep TypeScript running beside buffers without a name typescript-go stops when it opens a document whose URI is file://, which is what Neovim gives a buffer without a name (:enew, then a JavaScript or TypeScript filetype): `panic: vfs: path "tsconfig.json" is not absolute` (TypeScript 7.0.2). The buffer shares the project's tsc, so every TypeScript buffer of the project lost it, with "Client tsc quit with exit code 2". A nameless buffer got the working directory as its root since tsc came in with Neovim's LSP client (#113, 2026-09-27), in projects without TypeScript 6's tsserver.js, and in every project since tsc took them all (#170, 2026-10-03). tsc now stays off such buffers: its root_dir skips them and otherwise runs nvim-lspconfig's. That root_dir picks the binary its cmd starts, and each load of the config has its own choice, so cmd comes from the same load, as #113 did (a test with a project's own tsc checks it starts that one). CLAUDE.md says when to remove it. tsc also exits with code 1 after a clean shutdown, which Neovim reports on :lsp restart. That's left to typescript-go. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Link the upstream issue and fix for tsc's crash on file:// TypeScript 7's native port moved from microsoft/typescript-go, now archived, into microsoft/TypeScript, where the crash is microsoft/TypeScript#64300 and microsoft/TypeScript#64396 would fix it: CLAUDE.md now names the server by what it is and links both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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.
Some clients can send file URIs with an empty or relative path. Passing those through can produce bad filenames.
Normalize them using
vscode-uri-compatible rules, like the reference LSP implementation.I moved the implementation back into
lsprotosince the decoding eneds it, which makes this PR noisy, so ignore the last commit, I guess.Fixes #64300