fix(filesystem): sort directory_tree entries in stable lexicographic order

[jasonsuhari]

Closes #3539

Problem

fs.readdir returns entries in filesystem enumeration order, which is non-deterministic across platforms and runs. The directory_tree tool passed these entries directly into the result array without sorting, so identical directory contents could produce different output depending on host OS and filesystem.

Fix

Sort entries by name using localeCompare after each readdir call in buildTree, applied at every level of the recursive traversal:

const entries = (await fs.readdir(validPath, { withFileTypes: true }))
  .sort((a, b) => a.name.localeCompare(b.name));

No behaviour changes beyond ordering — no new parameters, no schema changes.

Tests

Applied the same fix to buildTreeForTesting in directory-tree.test.ts and added three new tests under a buildTree ordering describe block:

• Stable sibling order — asserts each name ≤ next name (lexicographically) at the root level
• Stable nested sibling order — same assertion on children of a subdirectory
• Idempotency — JSON.stringify of two consecutive calls produces identical output

All 149 filesystem tests pass.

[cliffhall]

@claude review

[claude]

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

---

Code Review

• Gather context and understand request
• Read changed files
• Analyze changes for correctness, style, and potential issues
• Post review feedback

---

The core filesystem fix is correct and well-motivated. However, there are a few issues worth addressing:

---

1. Unrelated changes bundled in the PR (Medium)

The PR description only mentions sorting directory_tree entries, but the PR also includes two commits adding tool annotations to the fetch server (c8bea37 feat(fetch): add tool annotations to server-fetch). These are unrelated changes that affect different packages. Mixing them makes the PR harder to review and revert independently. Ideally these should be separate PRs.

---

2. localeCompare without a fixed locale is still non-deterministic (Medium)

src/filesystem/index.ts:553 and src/filesystem/__tests__/directory-tree.test.ts:17

.sort((a, b) => a.name.localeCompare(b.name))

String.prototype.localeCompare() without arguments uses the runtime's default locale, which is determined by the host OS locale setting. This means the sort order of entries containing accented characters, hyphens, or mixed case could still differ between a macOS dev machine and a Linux CI runner.

For truly stable, cross-platform deterministic ordering, either pin a locale:

.sort((a, b) => a.name.localeCompare(b.name, 'en-US'))

Or use plain Unicode codepoint comparison (the most portable):

.sort((a, b) => (a.name < b.name ? -1 : a.name > b.name ? 1 : 0))

The latter is what git and many Unix tools use internally and is fully deterministic regardless of locale settings.

The same issue applies to buildTreeForTesting in the test helper. Fix this →

---

3. Missing newline at end of test file (Low)

src/filesystem/__tests__/directory-tree.test.ts — The file is missing a trailing newline (shown in the diff as \ No newline at end of file). Most editors and linters expect a final newline in text files. Fix this →

---

4. idempotentHint: true on the fetch tool is debatable (Low)

src/fetch/src/mcp_server_fetch/server.py:192

idempotentHint=True,

The MCP spec defines idempotentHint as "calling the tool repeatedly with the same arguments will have no additional effect on the system state." Since the fetch URL's response content can change between requests (dynamic pages, APIs with side effects like view counters), marking it idempotent may mislead clients about cacheability. readOnlyHint=True already captures the key safety property. This may be worth revisiting.

---

Summary

#	Severity	Issue
1	Medium	Fetch server annotations should be a separate PR
2	Medium	localeCompare() without locale is still non-deterministic across platforms
3	Low	Missing trailing newline in test file
4	Low	idempotentHint=True is semantically debatable for HTTP fetch

The tests added for ordering are well-structured and the idempotency test in particular is a nice addition. The core one-line change in index.ts is correct in intent — just worth pinning the locale to match the PR's stated goal of stable ordering.

[LuuOW]

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