Skip to content

fix(filesystem): wait for roots initialization before handling tool calls#3806

Draft
majiayu000 wants to merge 2 commits intomodelcontextprotocol:mainfrom
majiayu000:fix/filesystem-wait-for-roots-initialization
Draft

fix(filesystem): wait for roots initialization before handling tool calls#3806
majiayu000 wants to merge 2 commits intomodelcontextprotocol:mainfrom
majiayu000:fix/filesystem-wait-for-roots-initialization

Conversation

@majiayu000
Copy link
Copy Markdown

Fixes #3204

Description

Tool calls can arrive before oninitialized finishes fetching roots, so allowedDirectories is empty and every path check returns "Access denied". This adds a Promise-based gate that blocks tool handlers until roots are loaded.

The SDK fires oninitialized without awaiting it (() => this.oninitialized?.() in SDK index.js:53). The filesystem server's handler asynchronously calls server.server.listRoots(), but tool handlers run immediately against an empty allowedDirectories.

Server Details

  • Server: filesystem
  • Changes to: tools (initialization ordering)

Motivation and Context

When CLI args aren't provided and roots come from the client, there's a window where tool calls hit empty allowedDirectories. This is the race condition described in #3204.

How Has This Been Tested?

  • New unit tests in __tests__/roots-gate.test.ts covering: immediate resolution, delayed resolution, timeout, rejection, and concurrent waiters.
  • All 105 passing tests in the existing suite still pass. (lib.test.ts has a pre-existing vi.mock failure on main, unrelated to this change.)
  • npx tsc --noEmit passes clean.

Breaking Changes

None. If CLI args already provide allowed directories, the gate resolves immediately and there's no behavior change. The 10s timeout only applies when waiting for client-provided roots.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Protocol Documentation
  • My changes follows MCP security best practices
  • I have updated the server's README accordingly
  • I have tested this with an LLM client
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have documented all environment variables and configuration options

Additional context

What changed:

  1. roots-gate.ts (~25 lines) - createRootsGate(timeoutMs) factory returning { promise, resolve, waitForReady }. Races the internal promise against a configurable timeout.

  2. index.ts - imports the gate, resolves it immediately when CLI args provide directories, resolves it in oninitialized after roots load, and adds await gate.waitForReady() at the top of all 13 tool handlers.

  3. __tests__/roots-gate.test.ts - 5 test cases for the gate in isolation.

Add a Promise-based gate that blocks tool handlers until
allowedDirectories are initialized. This prevents a race condition
where tool calls arrive before oninitialized finishes fetching roots,
causing false "Access denied" errors.

If CLI args already provide directories the gate opens immediately.
Otherwise it opens when oninitialized completes (via try/finally).

Fixes modelcontextprotocol#3204

Signed-off-by: majiayu000 <1835304752@qq.com>
Extract the inline Promise gate into roots-gate.ts with a configurable
timeout (default 10s) to prevent indefinite hangs when oninitialized
never fires. Add unit tests for immediate resolution, delayed resolution,
timeout rejection, and concurrent waiters.

Signed-off-by: majiayu000 <1835304752@qq.com>
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.

Filesystem MCP: Server should wait inital roots to be loaded before handling tool calls

1 participant