fix(memory, sequentialthinking, filesystem): declare zod as a dependency (#4330)#4539
Open
gianghungtien wants to merge 1 commit into
Open
Conversation
The memory, sequentialthinking and filesystem servers all `import { z } from
"zod"` but never declared zod in their package.json. The import resolved only
by accident: the MCP SDK depends on zod, and npm's hoisted node_modules layout
happens to place it where these packages can reach it.
Under a strict node_modules layout (pnpm without hoisting, yarn PnP) the
phantom dependency is not reachable and every one of these servers dies on
startup:
Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'zod' imported from
.../@modelcontextprotocol/server-memory/dist/index.js
Declare zod explicitly in all three packages. The range `^4.0.0` matches the
everything server, which already declared it, and sits inside the SDK's
`^3.25 || ^4.0` peer range, so zod still dedupes to a single instance shared
with the SDK — schemas built here stay recognisable to the SDK.
Add a per-package regression test that scans the shipped sources for bare
import specifiers and asserts each one is declared, so a phantom dependency
cannot silently return.
Fixes modelcontextprotocol#4330
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
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
memory,sequentialthinkingandfilesystemallimport { z } from "zod"but none of them declaredzodinpackage.json. This PR declares it, and adds a regression test so a phantom dependency cannot silently return.Fixes #4330.
Server Details
memory,sequentialthinking,filesystempackage.jsondependencies (+ tests). No tool, resource, or prompt behaviour changes.Motivation and Context
Each of the three servers imports
zoddirectly:src/memory/index.ts:6src/sequentialthinking/index.ts:5src/filesystem/index.ts:13None declared it. The import resolves only by accident:
@modelcontextprotocol/sdkdepends onzod, and npm's hoistednode_moduleslayout happens to place it somewhere these packages can reach. That is a phantom dependency — it works until the layout changes.Under a strict layout (pnpm without hoisting, yarn PnP) it is not reachable, and the servers die on startup. Reproduced against the current published packages:
All three fail identically. The issue reports
memoryandsequentialthinking;filesystemhas the same defect and is included here.Note this is not only a strict-layout concern. The SDK's zod range is
^3.25 || ^4.0, so even under npm the servers silently bind to whichever major the SDK happens to hoist — a future SDK release could flip that under them.Why
^4.0.0everythingserver, the one TS server that already declaredzod, so the repo stays consistent.^3.25 || ^4.0peer range, sozodstill dedupes to one instance shared with the SDK. That matters: two zod copies would make schemas built here unrecognisable to the SDK'sinstanceofchecks. Verified after the fix — a singlezod@4.4.3is installed and linked.How Has This Been Tested?
Reproduction, before and after. Packed all three servers from this branch and installed the tarballs into a clean project using the same strict config that fails above (
node-linker=isolated,hoist=false):All three now start over stdio where all three previously aborted.
Regression test. Each package gets
__tests__/dependencies.test.ts, which scans the sources that compile intodist/for bare import specifiers and asserts each is declared independencies(relative paths and Node builtins are skipped, deep/scoped specifiers reduce to the package name). Confirmed it actually catches the bug by removingzodfromsrc/memory/package.jsonand re-running:It is written with vitest, per CONTRIBUTING.md.
Suites and build.
npm cifrom a clean tree (so the lockfile is verified the way CI installs it),npm run build, then per-packagenpm test:tsc --noEmitis clean for all three.Not tested with an LLM client. This change adds no tools and alters no schemas or behaviour — the servers either start or they do not, which is what the reproduction above covers end to end. Happy to run a client check if you would like one anyway.
Breaking Changes
None. No client configuration changes. Existing installs on hoisted npm layouts are unaffected, since they already resolve to the same zod v4 — the declaration just makes that guaranteed instead of incidental.
Types of changes
Checklist
Additional context
package-lock.jsonis regenerated to record the new declarations for the three workspaces; validated with a cleannpm ci.The regression test is duplicated per package rather than shared from a common helper, since each server is published standalone and its
tsconfig.jsonsetsrootDirto the package directory — reaching outside it would break the build. Tests are already excluded from compilation via**/*.test.ts, so nothing new ships indist/.Worth noting the same class of bug is easy to reintroduce elsewhere; the test is deliberately generic, so dropping it into another server would guard that one too.