fix(filesystem): preserve CLI directories when merging with MCP roots

[Bingtagui404]

Summary

Fixes #3602

When the MCP client supports the roots protocol, the oninitialized and roots/list_changed handlers unconditionally replaced all CLI-provided allowedDirectories with client roots. This caused any extra directories passed via command-line arguments to be silently discarded.

Before: npx @modelcontextprotocol/server-filesystem /home/user /mnt/Storage /mnt/Games + client root /home/user → only /home/user accessible.

After: CLI directories are preserved as an immutable baseline and merged with client roots. All four directories remain accessible.

Changes

• src/filesystem/index.ts: Save CLI directories as cliAllowedDirectories baseline. Change updateAllowedDirectoriesFromRoots to merge CLI baseline + current client roots instead of replacing. When roots become empty/invalid, fall back to CLI baseline only (no stale roots).
• src/filesystem/roots-utils.ts: Add mergeAllowedDirectories() helper — uses Set to deduplicate CLI dirs + root dirs.
• src/filesystem/__tests__/roots-utils.test.ts: Add 3 regression tests:
  • Preserves CLI directories while adding current client roots
  • Deduplicates directories shared by CLI args and client roots
  • Falls back to CLI baseline when client provides no valid roots

Test plan

• npx tsc --noEmit — type check passes
• npx vitest run --config src/filesystem/vitest.config.ts src/filesystem/__tests__/roots-utils.test.ts — all 6 tests pass (3 existing + 3 new)
• Full filesystem test suite — no new failures introduced

[Bingtagui404]

Filesystem checks are passing for this PR. The only failing job is Build fetch, which appears unrelated to the files changed here. I've pushed a no-op commit to retrigger CI.

[Bingtagui404]

Filesystem-related checks are passing for this PR. The failing Build fetch job is unrelated to this change: it fails during uv sync --locked --all-extras --dev because src/fetch/uv.lock needs to be updated. The same Build fetch failure is also present on the current main branch (run #23161383338).

[Bingtagui404]

To clarify: the Build fetch failure is a known repo-wide issue tracked in #3548. It is unrelated to the filesystem changes in this PR.

[cliffhall]

@claude review

[claude]

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

---

PR Review

• Gather context
• Read changed files
• Review logic in index.ts
• Review roots-utils.ts helper
• Review tests
• Post final review

---

Summary

This PR correctly fixes a real bug: when a client sends roots via the MCP protocol, the server was unconditionally replacing CLI-provided allowedDirectories with client roots. The fix is simple, well-targeted, and the tests are appropriate.

Verdict: Looks good overall with one stale comment to fix (minor).

---

What changed (diff summary)

File	Change
index.ts	Captures cliAllowedDirectories snapshot; updateAllowedDirectoriesFromRoots now calls mergeAllowedDirectories instead of replacing
roots-utils.ts	Adds mergeAllowedDirectories() helper — Set-deduped union of CLI dirs + root dirs
roots-utils.test.ts	3 new unit tests for mergeAllowedDirectories

---

Issue found

Stale comment at src/filesystem/index.ts:724 — the setNotificationHandler block still says:

// Handles dynamic roots updates during runtime, when client sends "roots/list_changed"
// notification, server fetches the updated roots and **replaces** all allowed directories
// with the new roots.

That describes the old behavior. The new behavior merges CLI dirs with the incoming roots rather than replacing. The comment should be updated to reflect that. Fix this →

---

Correctness analysis

cliAllowedDirectories capture point (index.ts:95)

allowedDirectories = accessibleDirectories;   // line 90 — filtered to accessible only

const cliAllowedDirectories = [...allowedDirectories];  // line 95 — snapshot taken here

Captured after accessibility filtering, so inaccessible CLI dirs aren't re-admitted through the merge path. Correct.

mergeAllowedDirectories (roots-utils.ts:85-90)

return [...new Set([...cliAllowedDirectories, ...rootDirectories])];

Simple, correct, and preserves CLI-first ordering (CLI dirs appear before root dirs in the result). The Set deduplicates without sorting, which is the right behavior.

Empty-roots fallback behavior

• Old code: when validatedRootDirs.length === 0, the function returned early without changing allowedDirectories, which meant any previously merged roots could linger.
• New code: mergeAllowedDirectories(cliAllowedDirectories, []) returns just the CLI baseline, cleanly evicting stale roots. This is a net improvement in the empty-roots case.

Symlink handling

cliAllowedDirectories is captured after the startup symlink-resolution loop that produces both original and resolved paths (e.g. /tmp and /private/tmp on macOS). Those entries are correctly preserved in the baseline.

---

Tests

The three new tests cover exactly the three contract properties of mergeAllowedDirectories:

1. CLI dirs preserved + roots added
2. Shared dirs deduplicated
3. Empty roots → CLI baseline only

All tests operate on the pure helper function rather than the full server setup, which is appropriate since the mutation logic in index.ts is straightforward enough not to require a separate integration test.

---

Minor notes (non-blocking)

• The pluralization logic in the log message (directory${... ? '' : 'ies'}) is correct but slightly non-idiomatic TypeScript; a directories alias or just always saying "directories" would simplify it.
• The PR description mentions "4 directories remain accessible" in the example but the example only lists 3 paths (plus the client root = 4 total). Worth a doc tweak but no code impact.

---

Overall this is a clean, minimal fix with good test coverage. The only actionable item before merge is updating the stale comment at line 724.

[LuuOW]

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