Repository navigation
test(filesystem): bring index.ts from 0% to 98.5% coverage (per #4474 server-by-server plan) - #4690
Parker-Fawcett wants to merge 2 commits into
Conversation
… writing literal filenames On POSIX, a path like C:\Users\me\notes\file.md is not absolute, so validatePath() sent it down the relative-path branch and path.resolve() placed it inside the allowed root as a single entry literally named 'C:\Users\me\notes\file.md' (or created a C:/ directory tree). Callers saw success and only discovered the mistake when inspecting disk - reported in modelcontextprotocol#4686. validatePath() now rejects drive-letter forms (^[A-Za-z]:(?:[\\/]|$)) up front when process.platform is not win32, with an explicit access-denied error. The check is one-directional and platform-guarded so Windows behavior is untouched and POSIX paths are never rewritten (no modelcontextprotocol#3628 regression). Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…, and roots flows index.ts was the only 0%-coverage file in server-filesystem: the entire MCP tool-registration layer (14 tools), every handler body, the argv-driven startup path, and the roots-protocol integration were untested. Per modelcontextprotocol#4474's per-server coverage plan, this adds a vitest suite that captures registerTool calls through a mocked McpServer so the real wiring (names, schemas, annotations) plus handler behavior are exercised without changing production code or exporting test-only helpers. Coverage of src/filesystem/index.ts: lines 0% -> 98.54%, statements 0% -> 98.51%, functions 0% -> 100%, branches 0% -> 94.28%. Server overall rises from 46% to 90.39%. Suite grows 158 -> 206 tests, all passing. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
cliffhall
added
the
closed-superseded-v2
|
Thank you for this pull request, and apologies for the long wait for a response. Since it was opened, the reference servers have been rebuilt for the 2026-07-28 MCP specification (#4857) and released as v2 (2.0.0 on npm for the TypeScript servers, 2026.10.19 on PyPI for the Python servers). The code this PR changes has been replaced, and the behavior it addresses has either been fixed in v2 or no longer exists in the same form. This is not a judgment on the change or on you; it's a consequence of the code it targets having been replaced. If the problem still happens on v2, please open a fresh issue with the version you're running and steps to reproduce. Note that this repository now accepts contributions as issues rather than pull requests from anyone but its maintainers; see This is part of the backlog triage in #4875. |
Description
Adds
src/filesystem/__tests__/index.test.ts(774 lines, 48 tests), covering the previously untestedsrc/filesystem/index.ts— the MCP tool-registration layer. Contributes the filesystem chunk of #4474 ("Write the missing tests, server by server — each server can be its own PR") and supplies data for its first task ("Audit current coverage per server").No production code is modified. The suite captures
registerTool(name, config, handler)calls through a mockedMcpServer, so the real wiring is asserted directly and handlers are invoked without exporting any test-only helpers or changing module structure.Server Details
Motivation and Context
#4474 asks for a ≥90% per-file coverage gate across servers. The audit baseline shows every TS server's
index.tsbootstrap/registration file sits at or near 0%, andserver-filesystemhad the largest such gap (~750 lines covering all 14 tools). This PR closes that gap for the repo's most-deployed reference server.Coverage audit baseline (current main, for the #4474 tracking comment):
What's covered
readOnlyHint/destructiveHint/idempotentHint) matching each tool's semantics.wx-flag call shape, edit_file apply vs dry-run, create_directory, list_directory, list_directory_with_sizes both sort modes + stat-failure fallback + summary, directory_tree recursion + glob/plain excludePatterns, move_file, search_files match/exclude/no-match, get_file_info formatting.vi.resetModules()), macOS-style symlink resolution to dual original/resolved entries, inaccessible-directory skipping, the all-inaccessible exit guard, and the fatal connect-failure path.How Has This Been Tested?
npm run buildstays clean; full workspace untouched outside the new test file.src/filesystem/index.ts: lines 0% → 98.54%, statements → 98.51%, functions → 100%, branches → 94.28%, clearing Bring all servers to the 90% per-file coverage gate; reintroduce the rule to AGENTS.md #4474's 90/90/90/90 gate. Server overall: 46.1% → 90.4%.Breaking Changes
None — test-only change.
Types of changes
Checklist
Additional context
Design notes for review:
fs/promisesis mocked wholesale following the existinglib.test.tspattern;lib.ts/path-utils.tsare intentionally not mocked so handler tests exercise real validation logic against the mock boundary.roots-utils.tsimportsfs.promises(from'fs'), so the'fs'partial mock shares the same promises stub — keeping roots flows deterministic without mocking roots-utils itself.argvbecause the real one runs at import time; it is ordered last sincevi.resetModules()duplicates captured registrations.