fix: preserve v1 library host compatibility - #392
Conversation
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe package retains MCP SDK v1 and Hono dependencies at runtime. Documentation preserves the v1 STDIO integration path and identifies the v2 factory path. Contract tests verify dependency placement, packed installations, and legacy consumer type checking. ChangesMCP v1 compatibility
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/mcp-v2-migration-contract.test.ts`:
- Around line 12-26: The package contract tests must enforce that
`@modelcontextprotocol/node` is dev-only. In
tests/mcp-v2-migration-contract.test.ts lines 12-26, add a negative assertion
that dependencies excludes `@modelcontextprotocol/node`; apply the same assertion
in tests/package-contract.test.ts lines 800-809 while preserving the existing
devDependencies checks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: effaccaf-ddbf-4e3f-9af9-ddb6cec90f9d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
CHANGELOG.mddocs/library-api.mdpackage.jsontests/mcp-v2-migration-contract.test.tstests/package-contract.test.tstests/release-config.test.ts
|
@coderabbitai review |
|
Addressed in exact-head commit 1d22a42: both package contracts now enforce that @modelcontextprotocol/node remains dev-only; the review thread is resolved and focused validation passes.
Summary
createMiftahRuntimeSTDIO host path through the 1.x linecreateMiftahServerFactoryas the preferred implementation pathReproduced SemVer blocker
A clean consumer declaring only published Miftah 1.0.0 compiled the documented
@modelcontextprotocol/sdk/server/stdio.jsintegration. The same consumer failed with TS2307 against the prior development tarball because the SDK dependency disappeared. This PR restores that documented 1.x contract without changing runtime internals.Security impact
The retained SDK accepts the patched
@hono/node-server2.x range. The package now carries exact@hono/node-server2.0.10 and Hono 4.12.34 runtime dependencies, and the installed-package test verifies those exact resolved versions. Both production-only and full npm audits report zero vulnerabilities.Validation
Fixes #391
Summary by CodeRabbit
Compatibility
Package Updates
Quality Assurance