test(memory): bring index.ts to 100% coverage on all dimensions (per #4474 server-by-server plan) - #4692
Open
Parker-Fawcett wants to merge 4 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>
Adds src/memory/__tests__/index.test.ts covering the tool-registration and wiring layer through a captured McpServer harness: all nine knowledge-graph tools' handlers against the real KnowledgeGraphManager over a mocked fs boundary (create/dedup, relation cascade deletes, observation add/remove/search/open semantics), the knowledge-graph resource read callback, subscribe/unsubscribe request handlers driving notifications/resources/updated on mutation (and silence after unsubscribe or for foreign URIs), non-ENOENT read-failure propagation versus empty-graph tolerance, and the fatal startup failure path via an injected transport fault. Coverage of src/memory/index.ts: lines/statements/functions/branches all 81-91% -> 100%. Server overall reaches 100%. Suite grows 50 -> 69 tests. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
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/memory/__tests__/index.test.ts(360 lines, 19 tests) takingsrc/memory/index.tsto 100% lines / statements / functions / branches. Third PR in the #4474 server-by-server series (filesystem #4690, sequentialthinking #4691).No production code changes. Handlers run against the real
KnowledgeGraphManagerwith only the fs boundary mocked, so persistence semantics (JSONL round-trips, dedup filters, cascade deletions) are verified end-to-end rather than against stubbed returns.Server Details
Motivation and Context
#4474 requires ≥90% per-file coverage on all four dimensions; memory's
index.tssat at 81-91% with all ten handler callbacks plus the fatal-startup path unexercised.What's covered
registerToolharness: create_entities persistence, create_relations dedup against stored state, add_observations (append + unknown-entity rejection), delete_entities with relation cascade, delete_observations selective strip, delete_relations exact-match filter, read_graph (populated + empty-file tolerance + non-ENOENT failure propagation), search_nodes across name/type/observations with touching-relation inclusion, open_nodes one-endpoint relation discovery.application/json; subscribe/unsubscribe request handlers drivingnotifications/resources/updatedafter mutations — and staying silent after unsubscribe or for foreign URIs.main()failure path via an injected transport fault and process-exit spy.How Has This Been Tested?
npm run buildclean; suite 69/69 passing (50 pre-existing + 19 new).src/memory/index.ts: 100 / 100 / 100 / 100 (lines/statements/functions/branches), up from 81.76/91.17/80/80. Server overall: 100%.Breaking Changes
None — test-only change.
Types of changes
Checklist
Additional context
With filesystem (#4690), sequentialthinking (#4691), and memory (this PR), three of the four TS servers now clear #4474's per-file gate at their registration layers. Remaining: everything server (57.9% overall, gaps in
server/andtransports/) — larger surface, happy to take it as a follow-up if this approach looks right.