Skip to content

fix(filesystem): improve roots diagnostics and add tilde path coverage#3414

Open
ahmetguness wants to merge 1 commit intomodelcontextprotocol:mainfrom
ahmetguness:fix/filesystem-roots-tilde-diagnostics
Open

fix(filesystem): improve roots diagnostics and add tilde path coverage#3414
ahmetguness wants to merge 1 commit intomodelcontextprotocol:mainfrom
ahmetguness:fix/filesystem-roots-tilde-diagnostics

Conversation

@ahmetguness
Copy link

Summary

Improves root URI diagnostics and adds explicit test coverage for tilde-prefixed path edge cases in the filesystem server.

This change does not modify the intended tilde expansion semantics. Instead, it:

  • Consolidates tilde expansion logic by reusing expandHome
  • Improves error diagnostics when resolving root URIs
  • Adds test coverage for literal tilde-prefixed directory names
  • Fixes a Windows CWD-dependent test case

Related to #3412


Details

1. Diagnostics Improvements

parseRootUri previously swallowed resolution errors silently.
This change adds descriptive logging when a root URI cannot be resolved, helping diagnose client-side configuration issues (e.g. invalid paths, inaccessible directories).

If no valid roots remain after validation, the error output now includes the received root URIs for easier debugging.

2. Tilde Handling Clarification

The existing expandHome behavior remains unchanged:

  • ~ and ~/... expand to the user's home directory
  • Literal patterns such as ~MyFolder or /path/~backup remain unchanged

The change removes duplicated tilde handling logic and centralizes it through expandHome.

3. Test Coverage

Adds explicit tests for:

  • ~MyFolder (literal name, not expanded)
  • ~user (not expanded)
  • Tilde in middle of path
  • file:// root URIs containing tilde-prefixed directories
  • Windows root directory behavior independent of CWD

All tests pass locally on Windows (151/151).


Type of Change

  • Bug fix (non-breaking)
  • Documentation update
  • Test coverage improvement

Breaking Changes

None.

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.

1 participant