fix: normalize MCP home-relative paths on all platforms#127
Merged
Conversation
Closes #125 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes MCP path handling by normalizing “operational paths” (workspace roots and subprocess paths) across platforms, with explicit support for home-relative inputs (~, ~/..., ~\..., and file:///~/...) so MCP behavior doesn’t depend on shell expansion.
Changes:
- Introduces
McpPathNormalizerto expand home-relative paths and normalize them viaPath.GetFullPath. - Updates MCP root URI conversion and CLI subprocess startup to use the new normalization behavior.
- Adds regression tests covering
file:///~/...root conversion and~path normalization.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/TALXIS.CLI.Tests/MCP/RootsServiceTests.cs | Adds regression coverage for file:///~/... resolving against the user profile. |
| tests/TALXIS.CLI.Tests/MCP/McpPathNormalizerTests.cs | Adds unit tests for ~ / ~/... / ~\... expansion and Uri.LocalPath-shaped /~/... inputs. |
| src/TALXIS.CLI.MCP/RootsService.cs | Uses McpPathNormalizer when converting file:// roots into a working directory path. |
| src/TALXIS.CLI.MCP/McpPathNormalizer.cs | New helper implementing home-relative expansion + normalization for MCP operational paths. |
| src/TALXIS.CLI.MCP/CliSubprocessRunner.cs | Normalizes working directory and resolved CLI host paths before launching subprocesses. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
Addressed the review comments in 8455b37. The new MCP path tests no longer assume UserProfile is always available; they now assert the same fallback behavior as production when the profile directory is empty. |
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.
Summary
~,~/...,~\\..., andfile:///~/...against the current user profileDetails
This fixes #125 by keeping
~handling inside the MCP host's execution boundary instead of relying on shell expansion behavior.The fix is applied in:
RootsService.ConvertFileUriToPathCliSubprocessRunnerMcpPathNormalizerhelperValidation
dotnet build TALXIS.CLI.sln --configuration Debugdotnet test tests/TALXIS.CLI.Tests/TALXIS.CLI.Tests.csproj --configuration Debug --filter "FullyQualifiedName~RootsServiceTests|FullyQualifiedName~McpPathNormalizerTests|FullyQualifiedName~LogRedactionFilterTests"dotnet test TALXIS.CLI.sln --configuration Debug --no-buildRepo scan
I scanned the repo for similar cross-platform path-boundary issues around:
Uri.LocalPath/ file URI conversionAssembly.Location/ process startup pathsProcessStartInfo.WorkingDirectory%USERPROFILE%I did not find another same-shape internal bug beyond the MCP root + subprocess path boundary fixed here.