Skip to content

Conversation

@daniel-lxs
Copy link
Member

@daniel-lxs daniel-lxs commented Jun 16, 2025

Description

Fixes #4739

This PR resolves the issue where VS Code magic variables like ${workspaceFolder} don't work in MCP configuration on Windows due to backslashes in paths causing invalid JSON.

Changes Made

  • Enhanced injectVariables function in src/utils/config.ts to normalize Windows paths
  • Added path normalization using path.posix.normalize to convert backslashes to forward slashes
  • Smart detection - only normalizes strings that contain path separators (\ or /)
  • Comprehensive test coverage for Windows path scenarios and non-path variables

Testing

  • All existing tests pass (19/19)
  • Added tests for Windows path normalization with backslashes
  • Added tests for mixed slash scenarios
  • Added tests to verify non-path variables remain unchanged
  • Manual testing completed:
    • Windows paths with ${workspaceFolder} now work correctly
    • Non-path variables (API keys, URLs, etc.) are unaffected
    • Cross-platform compatibility maintained

Verification of Acceptance Criteria

  • Windows paths work: ${workspaceFolder} with Windows paths no longer causes "Invalid MCP settings JSON format" error
  • Cross-platform compatibility: Forward slashes work on all platforms including Windows
  • No regression: Non-path variables continue to work as expected
  • Proper error handling: Invalid JSON errors are eliminated for valid path variables

Technical Details

Root Cause: The original injectVariables function performed direct string replacement without considering that Windows paths contain backslashes, which are escape characters in JSON.

Solution:

  1. Detect strings that look like paths (contain \ or /)
  2. Normalize them using path.posix.normalize to use forward slashes consistently
  3. Leave non-path strings unchanged

Why forward slashes: Windows supports forward slashes in paths, and they work consistently across all platforms without JSON escaping issues.

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Comments added for complex logic
  • No breaking changes
  • All tests passing
  • Type checking passes
  • Linting passes

Important

Normalize Windows paths in injectVariables to use forward slashes, fixing JSON issues in MCP configuration.

  • Behavior:
    • injectVariables in config.ts now normalizes Windows paths to use forward slashes.
    • Only strings with path separators are normalized.
    • Non-path variables remain unchanged.
  • Testing:
    • Added tests for Windows path normalization in config.spec.ts.
    • Tests cover backslashes, mixed slashes, and non-path variables.
  • Misc:

This description was created by Ellipsis for 3f2ede6. You can customize this summary. It will automatically update as commits are pushed.

@daniel-lxs daniel-lxs requested review from cte, jr and mrubens as code owners June 16, 2025 13:36
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Jun 16, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Review] in Roo Code Roadmap Jun 16, 2025
Copy link
Collaborator

@mrubens mrubens left a comment

Choose a reason for hiding this comment

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

Think we can use the existing toPosix for this?

@daniel-lxs
Copy link
Member Author

@mrubens
Thank you! I forgot that was a thing we had.

- Use path.posix.normalize to convert backslashes to forward slashes
- Only normalize strings that contain path separators
- Ensures cross-platform compatibility and prevents JSON parsing errors
- Add comprehensive tests for Windows path normalization
- Add tests to verify non-path variables remain unchanged
- Fixes issue where ${workspaceFolder} with Windows paths caused invalid JSON
@daniel-lxs daniel-lxs force-pushed the fix/issue-4739-vscode-magic-variables-mcp branch from 3f2ede6 to 4cbe740 Compare June 17, 2025 14:33
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 17, 2025
@mrubens mrubens merged commit 38c7f36 into main Jun 17, 2025
10 checks passed
@mrubens mrubens deleted the fix/issue-4739-vscode-magic-variables-mcp branch June 17, 2025 18:13
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 17, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Jun 17, 2025
valekseev pushed a commit to valekseev/Roo-Code that referenced this pull request Jun 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer PR - Needs Review size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

VS Code magic variables don't work in MCP configuration

4 participants