fix(filesystem): normalize path separators in search_files results

[ianliuy]

What's broken?

The filesystem server's search_files tool and directory_tree exclude patterns return inconsistent results across platforms. On Windows, glob patterns like **/*.ts fail to match because paths use backslashes internally.

Who is affected?

Anyone using the filesystem MCP server on Windows. Linux/macOS users are unaffected since path.sep is already /.

When does it trigger?

Every time search_files or directory_tree (with exclude patterns) is called on Windows. The glob patterns silently fail to match, returning incorrect results.

Where is the bug?

• src/filesystem/lib.ts — searchFilesWithValidation(), line computing relativePath
• src/filesystem/index.ts — buildTree(), line computing relativePath

Both use path.relative() which returns backslash-separated paths on Windows (e.g., src\index.ts), then pass them directly to minimatch which expects forward slashes.

Why does it happen?

path.relative() uses the OS-native separator (\ on Windows, / on Linux). minimatch is a glob library that operates on forward-slash paths. When backslash paths are passed to minimatch on Windows, patterns like **/*.ts don't match src\index.ts.

How did we fix it?

Normalize relativePath to forward slashes immediately after computing it:

// Before
const relativePath = path.relative(rootPath, fullPath);

// After
const relativePath = path.relative(rootPath, fullPath).split(path.sep).join('/');

This is applied in both locations. The fix is minimal (2 lines changed) and only affects the path format used for glob matching — the returned full paths from search_files remain OS-native.

How do we know it works?

All 57 related tests pass (lib.test.ts, directory-tree.test.ts, structured-content.test.ts including the search_files test). The .split(path.sep).join('/') pattern is a no-op on Linux/macOS (where path.sep is already /), so there's zero risk of regression on those platforms.

Fixes #3517

[cliffhall]

@claude review

[claude]

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

---

Code Review

• Gather context and read changed files
• Review the diff and implementation
• Post review feedback

---

Summary: This is a correct, minimal, low-risk fix for a real Windows-only bug. The approach is idiomatic and appropriate.

---

What the PR does

path.relative() returns OS-native separators — backslashes on Windows — but minimatch expects forward-slash paths for glob matching. Both changed lines feed the relativePath value directly into minimatch, causing patterns like **/*.ts to silently fail on Windows.

The fix in both locations:

// Before
const relativePath = path.relative(rootPath, fullPath);

// After
const relativePath = path.relative(rootPath, fullPath).split(path.sep).join('/');

On Linux/macOS path.sep === '/', so .split('/').join('/') is a no-op — zero regression risk on those platforms.

---

What's good

• Correct root cause diagnosis. path.sep is exactly the right constant to use here; it avoids hardcoding \\.
• Both sites addressed. The bug existed in searchFilesWithValidation (src/filesystem/lib.ts:393) for both the excludePatterns check and the main pattern match, and in buildTree (src/filesystem/index.ts:558) for the directory_tree exclude patterns. Both are fixed.
• Returned paths are unaffected. results.push(fullPath) in searchFilesWithValidation still uses the OS-native path, so the API contract for callers is unchanged.
• Idiomatic pattern. .split(path.sep).join('/') is the standard cross-platform normalization idiom for this purpose.

---

Suggestions

1. Missing test for the Windows path normalization scenario

The existing tests mock the filesystem at a POSIX level, so on Linux CI path.sep is always / and the new code paths are exercised but the normalization itself is never tested. There's no test that would have caught this bug before the fix, and there's none added that would catch a regression. Consider adding a unit test like this in lib.test.ts:

it('matches patterns correctly even when relativePath contains backslashes', async () => {
  // Simulate the Windows path.relative() output by mocking path.sep
  const originalSep = path.sep;
  Object.defineProperty(path, 'sep', { value: '\\', configurable: true });
  try {
    // ... set up mocks with backslash paths and assert minimatch matches correctly
  } finally {
    Object.defineProperty(path, 'sep', { value: originalSep, configurable: true });
  }
});

This would make the fix verifiable in CI on all platforms.

2. Consider path.posix.normalize or the slash convention

.split(path.sep).join('/') works correctly for this case but won't handle the rare edge case of mixed separators (e.g., src/components\Button). An alternative is the well-known slash package or a simple regex replace /\\/g, '/'. That said, path.relative() on Windows produces consistently backslash-only output, so this is not a practical concern here — the current approach is fine.

---

Verdict

The fix is correct and safe to merge. The only real gap is the lack of a platform-simulating test, but that's a "nice to have" rather than a blocker given how straightforward and well-understood the normalization is.