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

[ahmetguness]

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.

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 2m 24s —— View job

---

PR Review: fix(filesystem): improve roots diagnostics and add tilde path coverage

• Gather context
• Read changed files
• Analyze changes
• Post review feedback

---

Overall this is a clean, well-intentioned PR. The test additions are solid and the consolidation of tilde expansion logic is correct. I found one real correctness issue (double-logging) and a couple of minor concerns.

---

Root URI Diagnostics (roots-utils.ts)

Double-logging bug — this is the main concern.

parseRootUri now logs when it catches an error and returns null:

} catch (error) {
  const message = error instanceof Error ? error.message : String(error);
  console.error(`Failed to resolve root URI "${rootUri}": ${message}`);
  return null;
}

But the caller in getValidRootDirectories already logs a second message whenever resolvedPath is null:

if (!resolvedPath) {
  console.error(formatDirectoryError(requestedRoot.uri, undefined, 'invalid path or inaccessible'));
  continue;
}

So for any URI that fails to resolve, users now see two error lines — the specific one from parseRootUri, followed by the generic "Skipping invalid path or inaccessible" from the caller. This undercuts the diagnostics improvement the PR is targeting.

The fix is simple: since parseRootUri already provides the specific error, remove the redundant outer log:

if (!resolvedPath) {
  // parseRootUri already logged the specific failure reason
  continue;
}

Fix this →

---

Tilde Expansion Deduplication (roots-utils.ts)

The switch from the inline tilde check to expandHome is clean and correct — both paths produce identical behavior. The old code in parseRootUri was:

const expandedPath = rawPath.startsWith('~/') || rawPath === '~' 
  ? path.join(os.homedir(), rawPath.slice(1)) 
  : rawPath;

…which is exactly what expandHome does. This deduplication is the right call.

---

index.ts: Template Literal Reformatting

The list_directory_with_sizes handler has a minor cosmetic regression. The original was:

`${entry.isDirectory ? "[DIR]" : "[FILE]"} ${entry.name.padEnd(30)} ${
  entry.isDirectory ? "" : formatSize(entry.size).padStart(10)
}`

The new version puts everything on one long line with a dangling closing backtick, which is harder to scan. The original multi-line form was cleaner. This is cosmetic-only but worth reverting.

---

Windows Test Fix (path-validation.test.ts)

The change from:

expect(isPathWithinAllowedDirectories('D:\\other', ['/'])).toBe(false);

to dynamically picking a drive letter that differs from the CWD's drive is a correct fix. The original would fail on machines whose CWD is on D:, since / resolves to the current drive root on Windows.

---

Test Coverage

The new tests are well-structured:

• expandHome tests in path-utils.test.ts — good explicit coverage of the "literal tilde name" contract (~MyFolder, ~user, tilde-in-middle)
• tilde-prefixed directory names block in roots-utils.test.ts — correct integration tests using real tmpdir directories; uses toContain('~MyFolder') rather than an exact path match, which is portable
• startup-validation.test.ts — good end-to-end smoke test that exercises the full server spawn path with a ~MyFolder argument

One observation: the roots-utils.test.ts tilde tests only cover the plain-path and file:// URI cases. A test passing ~MyFolder (without a tmpdir prefix) as the URI root would hit the error path and could verify the new diagnostic logging — though that's optional given the scope of this PR.

---

JSDoc in path-utils.ts

The updated expandHome JSDoc is a clear improvement. Explicitly documenting that ~MyFolder is intentionally not expanded prevents future contributors from "fixing" it.

---

Missing EOF Newline

roots-utils.ts still ends without a trailing newline (\ No newline at end of file in the diff). Minor, but worth cleaning up for consistency.

---

Summary

Area	Verdict
Tilde deduplication via expandHome	✅ Correct, clean
parseRootUri error logging	✅ Good improvement
Double-logging in getValidRootDirectories	⚠️ Remove outer log
index.ts template literal reformat	⚠️ Minor cosmetic regression
Windows CWD-dependent test fix	✅ Correct
New test coverage	✅ Well-structured
EOF newline	ℹ️ Trivial

The double-logging issue is the only real change needed before merging.

[LuuOW]

Technical audit: Verified MCP server implementation for consistency with current SDK patterns.