Skip to content

feat: MCP SDK v2 migration with dual-era (2026-07-28 + legacy) support - #255

Merged
steipete merged 3 commits into
mainfrom
steipete/mcp2-sdk-v2
Aug 2, 2026
Merged

feat: MCP SDK v2 migration with dual-era (2026-07-28 + legacy) support#255
steipete merged 3 commits into
mainfrom
steipete/mcp2-sdk-v2

Conversation

@steipete

@steipete steipete commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Migrates mcporter from @modelcontextprotocol/sdk v1 to the SDK v2 packages (@modelcontextprotocol/client / @modelcontextprotocol/server 2.0.0), bringing full dual-era protocol support: the new stateless MCP revision 2026-07-28 and every legacy era the v2 SDK negotiates (2024-10-07 through 2025-11-25).

Highlights:

  • Auto version negotiation by default: server/discover probe with byte-identical legacy fallback. New per-server config field protocolVersion: "auto" | "legacy" | "2026-07-28" (schema regenerated).
  • src/sdk-patches.ts is gone. The prototype monkey-patches (child reaping, stderr capture, tracing) are replaced by a McporterStdioTransport subclass built on v2's public stderr/pid/teardown APIs. As a subclass, the negotiation probe runs in place on the live connection — no disposable sibling spawn per stdio connect (3s probe timeout).
  • serve bridge on v2 createMcpHandler: one HTTP endpoint serves 2026-07-28 clients and 2025-era clients side by side (stateless legacy fallback), stdio mode preserved, server__tool naming/escaping unchanged. New test drives the bridge with a client pinned to 2026-07-28.
  • Record/replay is era-aware: probe frames are recorded; pre-v2 recordings (which begin with initialize) automatically force legacy negotiation on replay, so all existing recordings keep working.
  • tools/list pagination now rides the SDK's aggregating walk; resources keep mcporter's bounded partial-result loop (the v2 walker throws at its cap, which would break mcporter's contract).
  • MRTR input_required results surface as a clear "requires interactive input" error (interactive fulfilment lands in the next PR).
  • list --verbose shows the negotiated protocol version/era. SDK v1 remains as a devDependency for legacy test fixtures only.

Proof: pnpm check clean; full suite 917 passed / 3 skipped; autoreview clean (0.96). Rebased over the repo tooling cleanup on main.

Part 2 of the MCP 2.0 series (after #254). Next: interactive elicitation/MRTR + OAuth hardening.

🤖 Generated with Claude Code

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Aug 2, 2026
@clawsweeper

clawsweeper Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed August 2, 2026, 4:48 PM ET / 20:48 UTC.

ClawSweeper review

What this changes

Migrates mcporter’s runtime and serving bridge to MCP SDK v2, adding automatic modern/legacy protocol negotiation, a per-server protocol-version override, and dual-era record/replay support.

Merge readiness

⚠️ Ready for maintainer review - 3 items remain

Keep open. This collaborator-authored PR remains necessary because current main and released v0.12.4 still use the v1 runtime, while the branch contains the SDK v2 migration; however, the migration leaves the upstream-tracking documentation pointing to a removed v1 shim and obsolete package version.

Priority: P2
Reviewed head: cc78f5b93d73a1524657a246eea21f366088e3aa

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The migration has substantial focused coverage and a coherent compatibility boundary, but a concrete post-migration troubleshooting documentation defect remains.
Proof confidence 🌊 off-meta tidepool Not applicable: This collaborator PR is not subject to the external-contributor proof gate; the supplied passing checks and focused tests are supplemental evidence, not independently re-run in this read-only review.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This collaborator PR is not subject to the external-contributor proof gate; the supplied passing checks and focused tests are supplemental evidence, not independently re-run in this read-only review.
Evidence reviewed 6 items Current main does not contain the migration: The SDK v2 branch head is not an ancestor of current main; current main still differs from the branch in the runtime transport, HTTP serving bridge, and package dependency files.
Protocol compatibility implementation: The branch maps auto, legacy, and pinned 2026-07-28 configuration to SDK v2 negotiation modes and caps stdio discovery probes at three seconds.
Focused compatibility coverage: The branch adds targeted transport tests for all three protocol modes and the stdio probe timeout, plus modern pinned HTTP bridge coverage and replay negotiation tests.
Findings 1 actionable finding [P2] Update obsolete SDK migration tracking docs
Security None None.

How this fits together

MCPorter turns configured stdio, HTTP, and SSE MCP servers into CLI and TypeScript runtime calls, then can expose daemon-managed tools through its own MCP serve bridge. This change sits at the transport boundary where configuration selects a client protocol path before tool calls, OAuth, recording, replay, and served responses continue downstream.

flowchart TD
  Config[Configured MCP server] --> Mode[Protocol mode selection]
  Mode --> Client[SDK v2 MCP client]
  Client --> Probe[Modern discovery or legacy fallback]
  Probe --> Remote[Remote MCP server]
  Remote --> Runtime[Tool and resource runtime]
  Runtime --> Bridge[CLI output and serve bridge]
Loading

Before merge

  • Update obsolete SDK migration tracking docs (P2) - The new runtime dependencies are @modelcontextprotocol/client and @modelcontextprotocol/server v2, but docs/hang-debug.md still calls SDK v1.21.0 current and tells users that workarounds live in the deleted src/sdk-patches.ts. Update that troubleshooting section with the v2 package boundary and current stdio guidance so debugging instructions do not send users to nonexistent code.
  • Resolve merge risk (P1) - This replaces the default MCP transport and negotiation layer for existing stdio, HTTP, SSE, OAuth, recording, and serve-bridge users; green unit and platform checks reduce risk but do not by themselves establish interoperability with deployed legacy servers.
  • Complete next step (P2) - A narrow, mechanical documentation repair can resolve the only identified blocking defect without changing the v2 migration design.

Findings

  • [P2] Update obsolete SDK migration tracking docs — package.json:73-74
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Migration surface 57 files affected; 813 added, 589 removed The branch changes runtime transports, OAuth imports, recording/replay, serving, configuration, dependency resolution, docs, and focused tests together.
Negotiation choices 3 modes: auto, legacy, pinned 2026-07-28 Existing configured servers use the new automatic default unless an operator explicitly pins a protocol era.

Merge-risk options

Maintainer options:

  1. Correct the v2 migration documentation (recommended)
    Update the hang-debug upstream-tracking section to describe the v2 client/server packages and remove references to the deleted sdk-patches.ts workaround before merge.
  2. Accept the release-validation risk
    Maintainers may land the migration after accepting that deployed legacy stdio and HTTP/SSE interoperability will be validated during the release process rather than by an additional live matrix in this PR.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Update docs/hang-debug.md for the SDK v2 client/server package split and remove references to deleted sdk-patches.ts; run pnpm docs:list, pnpm check, and pnpm test.

Technical review

Best possible solution:

Keep the v2 migration as the compatibility path, but correct all user-facing SDK-upstream tracking so it names the v2 package split and no longer directs debugging users to deleted local shims.

Do we have a high-confidence way to reproduce the issue?

Yes for the documentation defect: on the PR head, package.json installs v2 client/server runtime packages while docs/hang-debug.md still states that v1.21.0 is current and points to a deleted shim. Runtime interoperability was not executed in this read-only review.

Is this the best way to solve the issue?

No, not as submitted. The SDK v2 migration is aligned with MCPorter’s compatibility goal and has focused coverage, but the accompanying upstream-tracking documentation must describe the actual v2 dependency and debugging boundary.

Full review comments:

  • [P2] Update obsolete SDK migration tracking docs — package.json:73-74
    The new runtime dependencies are @modelcontextprotocol/client and @modelcontextprotocol/server v2, but docs/hang-debug.md still calls SDK v1.21.0 current and tells users that workarounds live in the deleted src/sdk-patches.ts. Update that troubleshooting section with the v2 package boundary and current stdio guidance so debugging instructions do not send users to nonexistent code.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 817843c5a4c0.

Labels

Label changes:

  • add P2: This is a broad but bounded compatibility migration with a concrete documentation defect and no evidence of an active outage.
  • add merge-risk: 🚨 compatibility: The default client, transport lifecycle, protocol negotiation, and package boundary change for existing configured MCP servers.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: This collaborator PR is not subject to the external-contributor proof gate; the supplied passing checks and focused tests are supplemental evidence, not independently re-run in this read-only review.

Label justifications:

  • P2: This is a broad but bounded compatibility migration with a concrete documentation defect and no evidence of an active outage.
  • merge-risk: 🚨 compatibility: The default client, transport lifecycle, protocol negotiation, and package boundary change for existing configured MCP servers.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: This collaborator PR is not subject to the external-contributor proof gate; the supplied passing checks and focused tests are supplemental evidence, not independently re-run in this read-only review.

Evidence

Acceptance criteria:

  • [P1] pnpm docs:list.
  • [P1] pnpm check.
  • [P1] pnpm test.

What I checked:

  • Current main does not contain the migration: The SDK v2 branch head is not an ancestor of current main; current main still differs from the branch in the runtime transport, HTTP serving bridge, and package dependency files. (src/runtime/transport.ts:1, cc78f5b93d73)
  • Protocol compatibility implementation: The branch maps auto, legacy, and pinned 2026-07-28 configuration to SDK v2 negotiation modes and caps stdio discovery probes at three seconds. (src/runtime/transport.ts:40, cc78f5b93d73)
  • Focused compatibility coverage: The branch adds targeted transport tests for all three protocol modes and the stdio probe timeout, plus modern pinned HTTP bridge coverage and replay negotiation tests. (tests/runtime-transport.test.ts:97, cc78f5b93d73)
  • Documentation regression: The branch makes @modelcontextprotocol/client and @modelcontextprotocol/server v2 runtime dependencies, but docs/hang-debug.md still says v1.21.0 is current and directs users to the deleted src/sdk-patches.ts workaround. (package.json:73, cc78f5b93d73)
  • Related migration history: The preceding merged replay/protocol-drift preparation work was introduced by the same current-main area contributor; this branch builds on that distinct merged preparation rather than duplicating it. (src/runtime/replay-transport.ts:1, 89fde9d16b69)
  • Release provenance: No release tag contains the unmerged SDK v2 branch head; v0.12.4 resolves to commit 437eab2986ce4ef6c42d5d4315c1c061ae53ec84, before this branch. (package.json:73, cc78f5b93d73)

Likely related people:

  • steipete: Current-main history attributes the transport/serve baseline and the merged protocol-drift replay preparation to Peter Steinberger, whose GitHub handle is supplied as steipete; the current branch continues that same compatibility work. (role: runtime and replay feature owner; confidence: high; commits: 437eab2986ce, 89fde9d16b69; files: src/runtime/transport.ts, src/runtime/http-transport.ts, src/serve.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Update docs/hang-debug.md to remove the v1-only version and deleted-shim instructions.
  • Before release, retain a redacted legacy and modern MCP interoperability transcript if one is available.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cc78f5b93d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/runtime.ts
Comment on lines +370 to +372
let response = await client.request(
{ method: 'resources/list', params: requestParams },
specTypeSchemas.ListResourcesResult

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve empty resource lists for tool-only servers

When a server does not advertise the resources capability, v2 Client.listResources() intentionally returns { resources: [] }, but this new no-cursor path skips that wrapper and sends resources/list through client.request(). That request is still capability-checked, so mcporter resource <server>/runtime.listResources(server) now errors on tool-only servers instead of reporting no resources. Please keep the first page on client.listResources(requestParams) or explicitly return an empty list when resources are unsupported, then only use the low-level request loop after a real first page with nextCursor.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant