fix(filesystem): fix quote style and add fileURLToPath regression test

[olaservo]

Summary

Follow-up to #3205 with two small improvements:

• Fix import quote style: Changed import { fileURLToPath } from "url" to use single quotes, matching the rest of the file
• Add regression test: New test using pathToFileURL() to verify properly-encoded file:// URIs (the format VS Code sends via MCP roots) are handled correctly on all platforms

Test plan

• npx vitest run src/filesystem/__tests__/roots-utils.test.ts — all 4 tests pass (3 existing + 1 new)

🦉 Generated with Claude Code

[koic]

It appears that the tests pass on non-Windows platforms regardless of the changes in #3205. Since CI also does not seem to run tests on Windows, how about converting this into a regression test that can be reproduced on macOS and other platforms as shown below?

Suggested change

	it('should handle standard file URIs from pathToFileURL', async () => {
	// pathToFileURL produces properly-encoded URIs (e.g. file:///C:/... on Windows)
	// This is the format VS Code and other editors send via MCP roots
	const roots: Root[] = [
	{ uri: pathToFileURL(testDir1).href, name: 'Standard File URI' },
	{ uri: pathToFileURL(testDir2).href, name: 'Standard File URI 2' },
	];
	const result = await getValidRootDirectories(roots);
	expect(result).toContain(testDir1);
	expect(result).toContain(testDir2);
	expect(result).toHaveLength(2);
	});
	it('should handle standard file URIs from pathToFileURL', async () => {
	// pathToFileURL encodes special characters (e.g. spaces become %20, # becomes %23).
	// fileURLToPath() decodes them correctly; slice(7) does not.
	// e.g. pathToFileURL('/tmp/my dir').href => 'file:///tmp/my%20dir'
	// fileURLToPath('file:///tmp/my%20dir') => '/tmp/my dir' (correct)
	// 'file:///tmp/my%20dir'.slice(7) => '/tmp/my%20dir' (wrong)
	const dirWithSpaces = join(testDir1, 'dir with spaces');
	const dirWithHash = join(testDir1, 'dir#hash');
	mkdirSync(dirWithSpaces);
	mkdirSync(dirWithHash);
	const roots: Root[] = [
	{ uri: pathToFileURL(dirWithSpaces).href },
	{ uri: pathToFileURL(dirWithHash).href },
	];
	const result = await getValidRootDirectories(roots);
	expect(result).toContain(dirWithSpaces);
	expect(result).toContain(dirWithHash);
	expect(result).toHaveLength(2);
	});

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 1m 4s —— View job

---

PR Review

• Read changed files
• Analyze changes
• Post review feedback

---

This is a clean, focused PR. Here's my assessment of both changes:

src/filesystem/roots-utils.ts — Quote style fix

Simple consistency fix: "url" → 'url'. Correct and unambiguous. ✅

src/filesystem/__tests__/roots-utils.test.ts — Regression test

The test is a step in the right direction, but it doesn't actually serve as a regression test for the bug fixed in #3205.

The core issue in #3205 was that slice(7) on a file URI doesn't decode percent-encoded characters, while fileURLToPath() does. For example:

file:///tmp/my%20dir    (space encoded as %20)
  slice(7)        → /tmp/my%20dir   ❌ (wrong)
  fileURLToPath() → /tmp/my dir     ✅ (correct)

The current test uses pathToFileURL(testDir1) where testDir1 is a mkdtempSync-generated path like /tmp/mcp-roots-test1-Xz4aFg. This path contains no special characters, so pathToFileURL produces a URI like file:///tmp/mcp-roots-test1-Xz4aFg — identical to what naive slice(7) would produce. The test would pass even if fileURLToPath were reverted to slice(7).

The review comment from @koic correctly identifies this and proposes a stronger version that creates subdirectories containing spaces and # characters, which would actually fail under the old buggy slice(7) implementation.

I'd recommend adopting koic's suggestion to make this a genuine regression test. It's a small change: create two subdirs (e.g. dir with spaces, dir#hash) inside the existing testDir1, then use pathToFileURL on those paths. Since these subdirs live inside testDir1, they'll be cleaned up automatically by the existing afterEach. Fix this →

Minor nit: The comment // pathToFileURL produces properly-encoded URIs (e.g. file:///C:/... on Windows) focuses on Windows path format, but the key value of fileURLToPath is percent-decoding. A comment like:

// pathToFileURL percent-encodes special characters (spaces → %20, # → %23).
// fileURLToPath() decodes them correctly; slice(7) does not.

...would better document why fileURLToPath is required.

Summary

Change	Assessment
Quote style fix	✅ Approve
Regression test	⚠️ Functionally correct but doesn't exercise the encoding bug — koic's suggestion makes it a real regression test

The PR is mergeable as-is (both changes are correct), but incorporating koic's suggestion before merge would meaningfully strengthen the test coverage.

[LuuOW]

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

[ChrisKrawczyk]

Hit this from a brand-new MCP host (Obsidian plugin) using pathToFileURL(<vault path>).href against @modelcontextprotocol/server-filesystem@2026.1.14. The slice(7) regression in roots-utils.ts makes every Windows vault path with a space (OneDrive folders, Program Files, etc.) fail with:

Skipping invalid path or inaccessible: file:///C:/Users/.../OneDrive%20-%20Microsoft/Vaults/copilot-test
No valid root directories provided by client

Confirmed root cause: slice(7) keeps the leading / before the drive letter and never decodes %20, so path.resolve produces C:\C:\...%20... which fs.realpath rejects with ENOENT. The fix in this PR (fileURLToPath) is correct — verified locally that all common Node-emitted file URIs round-trip cleanly.

+1 for merge — would unblock a class of Windows users whose vault sits anywhere with a space in the path (very common: OneDrive, Documents, etc.).