refactor!: extract LegacyServer/LegacyClient; NEW Server/Client compose them - #2134
Closed
felixweinberger wants to merge 6 commits into
Closed
felixweinberger wants to merge 6 commits into
felixweinberger wants to merge 6 commits into
Claude / Claude Code Review
completed
May 21, 2026 in 6m 37s
Code review found 2 important issues
Found 5 candidates, confirmed 3. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 2 |
| 🟡 Nit | 0 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | packages/client/src/client/legacyClient.ts:242-253 |
LegacyClient._isStatelessConnection never reset on close/reconnect |
| 🔴 Important | docs/migration.md:906-910 |
Migration table omits Server members removed in this PR (oninitialized, getClientCapabilities, getClientVersion, createE |
Annotations
Check failure on line 253 in packages/client/src/client/legacyClient.ts
claude / Claude Code Review
LegacyClient._isStatelessConnection never reset on close/reconnect
`LegacyClient._isStatelessConnection` is set to `true` in `_setNegotiated()` but is never reset to `false` — not in `Protocol.close()`, not in a `LegacyClient.close()` override, and not in `_initialize()`. After `connect(2026 server)` → `close()` → `connect(pre-2026 server)`, the legacy session is fully established but `_assertSession()` still sees the stale flag and `client.legacy.ping()` / `subscribeResource()` / `unsubscribeResource()` / `sendRootsListChanged()` incorrectly throw `SdkError(Se
Check failure on line 910 in docs/migration.md
claude / Claude Code Review
Migration table omits Server members removed in this PR (oninitialized, getClientCapabilities, getClientVersion, createElicitationCompletionNotifier, setNotificationHandler)
The new "Removed from Server/Client / Moved to" migration table omits several public Server members that this PR also removes (the PR's own tests/examples had to migrate them to `server.legacy.*`): `oninitialized`, `getClientCapabilities()`, `getClientVersion()`, `createElicitationCompletionNotifier()`, and `setNotificationHandler`/`fallbackNotificationHandler`. Add the missing rows, fix the now-stale `server.setNotificationHandler('notifications/message', …)` v2 example at docs/migration.md:~36
Loading