Skip to content

Conversation

@ntotten
Copy link
Member

@ntotten ntotten commented Nov 28, 2025

Supersedes #3803

Copilot AI review requested due to automatic review settings November 28, 2025 19:12
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds handling for file URLs (paths starting with file://) in the plugin resolution logic. The change prevents the extension from attempting to resolve file:// URLs as node modules, treating them similarly to relative paths (starting with .) and absolute paths.

Key Changes:

  • Modified resolveConfigPlugins to skip node module resolution for plugin paths starting with file://

Comment on lines +37 to +38
!path.isAbsolute(plugin) &&
!plugin.startsWith("file://")
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

The new condition to skip module resolution for file:// URLs lacks test coverage. Consider adding a test case in src/test/suite/plugins.test.ts or creating a new test fixture that includes a plugin path starting with file:// to ensure this behavior is properly tested and doesn't regress in the future.

Copilot uses AI. Check for mistakes.
!plugin.startsWith(".") &&
!path.isAbsolute(plugin)
!path.isAbsolute(plugin) &&
!plugin.startsWith("file://")
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

The check !plugin.startsWith("file://") is case-sensitive. While URL schemes are typically lowercase, RFC 3986 specifies that schemes are case-insensitive. Consider using !plugin.toLowerCase().startsWith("file://") to handle edge cases like FILE:// or File://.

Suggested change
!plugin.startsWith("file://")
!plugin.toLowerCase().startsWith("file://")

Copilot uses AI. Check for mistakes.
@ntotten ntotten merged commit 8cfaab2 into main Nov 28, 2025
19 checks passed
@ntotten ntotten deleted the handle_file_paths branch November 28, 2025 19:18
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.

2 participants