test(sequentialthinking): bring index.ts from 0% to full coverage (per #4474 server-by-server plan) - #4691
Open
Parker-Fawcett wants to merge 3 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>
Adds src/sequentialthinking/__tests__/index.test.ts covering the tool-registration layer that previously had no tests: the sequentialthinking tool wiring (annotations, description, outputSchema keys), the safe boolean-coercion preprocess across boolean/string/case-variant/passthrough inputs, z.coerce.number behavior on required fields, the handler's structured-content parsing and error passthrough paths, and the fatal startup failure path via a transport-connect fault injected on a fresh module evaluation. Coverage of src/sequentialthinking/index.ts: lines 0% -> 100%, statements -> 100%, functions -> 100%, branches -> 90%. Server overall rises from 54.7% to 98.11%. Suite grows 14 -> 27 tests. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds
src/sequentialthinking/__tests__/index.test.ts(198 lines, 13 tests) covering the previously untestedindex.ts— the tool-registration layer. Second PR in the #4474 server-by-server series (filesystem landed as #4690; memory is next).No production code changes.
lib.tsis mocked here so the handler's branch logic is driven deterministically; it keeps its own dedicated suite at 96.7%.Server Details
Motivation and Context
#4474 requires ≥90% per-file coverage on all four dimensions. The audit in #4690 showed this server's
index.tsat 0% (~134 lines: registration, schema coercion, handler, startup).What's covered
sequentialthinkingtool, read-only annotation set, detailed description, all fiveoutputSchemakeys.coercedBoolean): boolean passthrough,"true"/"TRUE"/"false"/"False"strings, and rejection of non-boolean non-string values through the real zod schema captured from the registration config."3","5") parse to numbers for required counters.structuredContentand passes content through untouched;isErrorresults are returned without parsing.How Has This Been Tested?
npm run buildclean; suite 27/27 passing (14 pre-existing + 13 new).src/sequentialthinking/index.ts: lines 0% → 100%, statements → 100%, functions → 100%, branches → 90% — clears Bring all servers to the 90% per-file coverage gate; reintroduce the rule to AGENTS.md #4474's gate. Server overall: 54.7% → 98.11%.Breaking Changes
None — test-only change.
Types of changes
Checklist
Additional context
One implementation note for reviewers: mocking a sibling module from a test inside
__tests__/requires the mock path relative to the test file (vi.mock('../lib.js')), not relative to the importing module — worth knowing if more per-server index suites follow this template.Next in the series per #4474: memory (81.8% overall, closest to its gate).