feat: add MCP server manifest - #574
Conversation
|
Queued — the merge queue status continues in this comment ↓. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #574 +/- ##
==========================================
+ Coverage 91.57% 92.03% +0.46%
==========================================
Files 31 33 +2
Lines 1270 1344 +74
Branches 317 335 +18
==========================================
+ Hits 1163 1237 +74
Misses 48 48
Partials 59 59 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Unit Test Results 1 files 29 suites 1s ⏱️ Results for commit c965c7f. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
✨ PR Review
The PR introduces a well-structured MCP server manifest with solid sync/check tooling and good test coverage. One concrete fragility exists in how the changesets version script invokes the CLI binary.
2 issues detected:
🧹 Maintainability - `node ./node_modules/@changesets/cli/bin.js` hardcodes an internal file path within a dependency, which is not part of the package's public API surface and can break on any internal refactor of `@changesets/cli`. 🛠️
Details: The version:changesets script directly invokes node ./node_modules/@changesets/cli/bin.js version instead of using the conventional node_modules/.bin/changeset version or npx changeset version. Accessing an internal package file path bypasses the package's own bin-entry contract and will silently break if the @changesets/cli package reorganizes its internals or moves the bin entrypoint.
File: package.json (53-53)
🛠️ A suggested code correction is included in the review comments.
🐞 Bug - When `drift` is non-empty but mutation + re-serialization produces bytes identical to the original file, the function returns `changed: false` alongside a non-empty `drift` array, which is a self-contradictory result. 🛠️
Details: In sync mode, after applying drift fixes the function computes updatedContents and writes the file only when updatedContents !== contents. The return value { changed: updatedContents !== contents, drift } is computed AFTER the conditional write. If the serialized result somehow equals the original on-disk content despite reported drift (e.g., a key ordering coincidence after mutation), the function returns { changed: false, drift: [...non-empty...] } — a logically contradictory result that would mislead callers and the CLI output ("server.json is synchronized") while drift entries are non-empty.
File: scripts/server-manifest.mjs (159-163)
🛠️ A suggested code correction is included in the review comments.
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how
|
Repaired Validated with — charlied/pr-check-repair |
There was a problem hiding this comment.
✨ PR Review
The PR adds well-structured MCP manifest tooling with solid validation and test coverage. The main concern is a sequencing bug in the manual version scripts where server.json sync occurs after npm version has already committed, leaving the manifest update uncommitted.
1 issues detected:
🐞 Bug - `npm version patch` commits `package.json` first; the subsequent `sync:server-manifest` call modifies `server.json` after the commit has already been created, leaving an uncommitted change. 🛠️
Details: The version:patch, version:minor, and version:major scripts run npm run sync:server-manifest after npm version, but npm version already creates a git commit (and tag) before the && chain continues. As a result, server.json is updated on disk but the change is left uncommitted — the version bump commit contains only package.json while server.json drifts behind in the working tree.
File: package.json (54-56)
🛠️ A suggested code correction is included in the review comments.
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how
|
@CharlieHelps gitStream AI review raised 3 issues that appear unaddressed:
|
|
Addressed and verified all three against the current PR head:
Focused tests and the full required local validation passed. All GitHub checks on the new head are terminal with no failures. |
Merge Queue Status
This pull request spent 2 minutes 34 seconds in the queue, including 1 minute 32 seconds running CI. Required conditions to merge
|
Summary
mcpName, a schema-valid rootserver.json, and requiredHEVY_API_KEYconfiguration.Validation
npm run check:server-manifestnpm run check:typesnpm run checknpm run buildnpx vitest run --exclude 'tests/integration/**'npm run check:changesetnpm pack --dry-run --jsonResolves #564
✨ PR Description
Purpose: Implement MCP server manifest generation and synchronization to expose registry metadata and ensure consistency between package.json and server.json.
Main changes:
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Description using Guidelines Learn how