Skip to content

Fix invalid URI for untitled references/definitions #1344

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jul 1, 2025

This PR addresses the issue where find references and go-to-definition operations on untitled files return incorrect URIs that cannot be linked back to the original location.

Root Cause Analysis

The issue was in the path handling logic in internal/tspath/path.go. When VS Code sends requests with untitled URIs like untitled:Untitled-2, the following sequence occurs:

  1. DocumentURIToFileName correctly converts untitled:Untitled-2 to ^/untitled/ts-nul-authority/Untitled-2
  2. GetEncodedRootLength did not recognize ^/ as a rooted path, returning 0
  3. ToPath treated the untitled filename as relative and resolved it against current directory
  4. ❌ This corrupted ^/untitled/ts-nul-authority/Untitled-2 into /home/user/project/^/untitled/ts-nul-authority/Untitled-2
  5. ❌ Later when FileNameToDocumentURI was called, it couldn't recognize the corrupted path as untitled and returned a regular file URI

Solution

Modified GetEncodedRootLength to recognize paths starting with ^/ as rooted paths (similar to how / and c: are handled). This prevents untitled filenames from being resolved against the current directory.

Implementation

  • Added check for ^/ prefix in GetEncodedRootLength function
  • Returns root length of 2 for untitled paths (length of ^/)
  • Added comprehensive tests for edge cases and round-trip conversion
  • Added integration test simulating the complete LSP flow
  • Verified existing functionality is preserved

Before Fix

Input: untitled:Untitled-2
File path after ToPath: /home/user/project/^/untitled/ts-nul-authority/Untitled-2  
Output URI: file:///home/user/project/%5E/untitled/ts-nul-authority/Untitled-2

After Fix

Input: untitled:Untitled-2
File path after ToPath: ^/untitled/ts-nul-authority/Untitled-2
Output URI: untitled:Untitled-2

The integration test confirms the complete flow works correctly, simulating the exact scenario from the original issue where references and definitions on untitled files now return proper untitled: URIs instead of broken file:// URIs.

Fixes #1343.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 3 commits July 1, 2025 23:59
Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com>
The issue was that untitled filenames (^/untitled/ts-nul-authority/...) were being treated as relative paths and resolved against the current working directory, corrupting their format.

- Modified GetEncodedRootLength to recognize ^/ as a rooted path prefix
- Added comprehensive tests for untitled path handling
- Verified fix resolves the URI conversion round-trip issue

Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com>
This test simulates the exact issue scenario:
- VS Code sends untitled:Untitled-2 URI
- Verifies the path is not corrupted by current directory resolution
- Confirms round-trip URI conversion works correctly
- Proves references/definitions will return correct untitled URIs

Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Invalid uri for untitled references/definitions Fix invalid URI for untitled references/definitions Jul 2, 2025
@Copilot Copilot AI requested a review from DanielRosenwasser July 2, 2025 00:18
Copilot finished work on behalf of DanielRosenwasser July 2, 2025 00:18
Comment on lines 167 to 171
// Untitled paths (e.g., "^/untitled/ts-nul-authority/Untitled-1")
if ch0 == '^' && ln > 1 && path[1] == '/' {
return 2 // Untitled: "^/"
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is uncommon, so move it right before the URL checks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the untitled path check to before the URL checks as requested. This improves performance by checking common cases (POSIX/DOS paths) first before the uncommon untitled paths. Fixed in 0fd6461.

Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com>
@Copilot Copilot AI requested a review from DanielRosenwasser July 2, 2025 00:50
Copilot finished work on behalf of DanielRosenwasser July 2, 2025 00:50
@DanielRosenwasser
Copy link
Member

There's a check for && !strings.HasPrefix(opts.FileName, "^/") somehwere in the code that no longer applies

Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jul 2, 2025

There's a check for && !strings.HasPrefix(opts.FileName, "^/") somehwere in the code that no longer applies

Removed the redundant check as requested. Since GetEncodedRootLength now returns 2 for ^/ paths instead of 0, the && !strings.HasPrefix(opts.FileName, "^/") condition is no longer needed. Fixed in 7d3c7e7.

Copilot finished work on behalf of DanielRosenwasser July 2, 2025 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid uri for untitled references/definitions
2 participants