Fix: TS server version drift - #4608
Draft
Medsaad wants to merge 6 commits into
Draft
Conversation
…json Athough this one was is in sync with package.json, it should get updated manually in the future.
6 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
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
Follow-up to #4576, as suggested by @KarlLeen there.
filesystem,memory, andeverythinghardcode a version literal in theirMcpServerconstructor .. the same defect #4575 reports forsequentialthinking. Those literals cannot be kept correct, becausescripts/release.pyrewritespackage.json["version"]at release time, so every server eventually misreports its version over the wire.This replaces each literal with
SERVER_VERSION, read from the package's ownpackage.jsonat runtime.package.json0.2.00.6.32.0.0memoryandeverythingmatch at rest but drift the moment a release stamps a CalVer version into their manifests, so all three are fixed together.sequentialthinkingis deliberately untouched here .. it belongs to #4576.Server Details
serverInfo.version(no tools, resources, or prompts changed)Motivation and Context
Related to #4575.
scripts/release.py:70-76(NpmPackage.update_version) rewritespackage.json["version"]to a date-based version generated bygen_version()at release time. A version committed in TypeScript source is therefore stale by construction .. it is never the version that ships. That makes this one systemic defect rather than a series of independent oversights, and it is why the fix has to read the manifest at runtime rather than sync a literal.This stays correct if #4463 (semver via changesets) changes how versions are stamped, since it reads whatever ends up in the shipped manifest.
Implementation
version.tsis duplicated at each package root rather than extracted into a shared package. A sharedsrc/shared/workspace package would be auto-detected as publishable by bothfind_changed_packages(release.py) and the CIdetect-packagesmatrix, would need its own npm trusted-publisher registration perRELEASING.md, and would make each server's release depend on a separately-versioned package. That is disproportionate for ~30 lines, so the duplication is deliberate.The implementation matches #4576 deliberately, so all four servers end up with the same file. Two details worth noting for review:
dist/). Those two locations are checked and nothing else, it never walks up into the monorepo and cannot pick up a parentpackage.json.version.tsmust stay at the package root. Moving it into a subdirectory breaks thedist/case. This matters foreverything, where the surrounding code is organised into subfolders.There is one intentional difference from #4576:
A blanket
catchswallows every failure, so a corrupt or permission-deniedpackage.jsonreportsCould not locate package.json for server version.. misleading for a file that was found and is unreadable for a different reason. Narrowing toMODULE_NOT_FOUNDlets real failures surface. Happy to drop this if reviewers would rather the four files stay byte-identical, and equally happy to open it against #4576 instead.How Has This Been Tested?
npm run buildclean across all packages (everythingtype-checks its tests, and passes).sequentialthinkingunchanged at 14.Each package's
version.tsgets six tests, in two groups:createRequiremocked: the happy path against the real manifest, throwing when neither candidate exists (asserting the search stops at the package root), propagation of non-MODULE_NOT_FOUNDerrors (EACCES), and propagation of a malformed manifest. The mock is a pass-through by default, so the happy-path assertion runs against the real filesystem in the same file; only tests that need a failure inject one.dist/version.js, and spawningnode dist/index.jsto readserverInfo.versionback over a stdio handshake (everything2.0.0,filesystem0.6.3,memory0.6.3;filesystemreported0.2.0before this change). These useit.skipIfso an unbuilt tree still passes, matching fix(sequentialthinking): read server version from package.json #4576. They do run in CI, sincenpm citriggersprepare..npm run build.The two propagation tests and the bounded-search test were mutation-tested: reverting the catch to a blanket
catch {}fails the first two, and adding a third candidate path beyond the package root fails the third.Breaking Changes
None. No client configuration changes. Servers now report their actual published version instead of a stale literal, which is the intent of the fix.
Types of changes
Checklist
Additional context
This overlaps existing work, and I'd rather flag that up front than have a reviewer find it:
sequentialthinking. This PR is the follow-up @KarlLeen proposed there and stays out of its way.memoryandsequentialthinking(alongside a zod dependency fix). If that lands first, I'll dropmemoryhere and reduce this tofilesystemandeverything.memory, was closed as not planned.If maintainers would rather have one combined change, I'm happy for this to be folded into #4576 instead. Credit to @KarlLeen for reporting #4575 and to the authors of #4576 and #4557 .. the direction here is the same as theirs.