feat(core): add custom request/notification handler API to Protocol#1846
Closed
felixweinberger wants to merge 13 commits into
Closed
feat(core): add custom request/notification handler API to Protocol#1846felixweinberger wants to merge 13 commits into
felixweinberger wants to merge 13 commits into
Claude / Claude Code Review
completed
Apr 13, 2026 in 6m 21s
Code review found 2 potential issues
Found 5 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | packages/core/test/shared/customMethods.test.ts:232-237 |
Dead error-capture setup in no-handler notification test |
| 🟡 Nit | examples/server/src/customMethodExample.ts:19 |
Examples use bare zod import instead of zod/v4 |
Annotations
Check warning on line 237 in packages/core/test/shared/customMethods.test.ts
claude / Claude Code Review
Dead error-capture setup in no-handler notification test
This test sets up `const errors: Error[] = []; client.onerror = e => errors.push(e);` but never asserts on `errors`, so the capture is dead code — and it's attached to the *receiver* (`client`), not the sender the test name references. Either drop the two lines, or add `expect(errors).toEqual([])` after a tick if you want to assert the receiver stays quiet too.
Check warning on line 19 in examples/server/src/customMethodExample.ts
claude / Claude Code Review
Examples use bare zod import instead of zod/v4
nit: these two new example files use `import { z } from 'zod'` while every other example in the repo (and the SDK internals) uses `import * as z from 'zod/v4'`. Consider switching to `import * as z from 'zod/v4'` here and in `examples/client/src/customMethodExample.ts:13` for consistency with the codebase convention.
Loading