Skip to content

merge commit

ea0e584
Select commit
Loading
Failed to load commit list.
Open

PoC: TS exactOptionalPropertyTypes support #1821

merge commit
ea0e584
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Apr 11, 2026 in 17m 7s

Code review found 2 important issues

Found 5 candidates, confirmed 4. See review comments for details.

Details

Severity Count
🔴 Important 2
🟡 Nit 0
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important packages/server/src/server/mcp.ts:1128-1137 Incomplete | undefined additions force unsafe as-casts in three places
🔴 Important packages/server/src/server/server.ts:116-125 Server constructor conditional spread introduces unintended behavioral regression

Annotations

Check failure on line 1137 in packages/server/src/server/mcp.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

Incomplete | undefined additions force unsafe as-casts in three places

Three hand-written SDK files were not updated for `exactOptionalPropertyTypes`, leaving unsafe `as`-casts in `mcp.ts:1133`, `client.ts:714`, and `server.ts:425`, plus verbose conditional-spread workarounds in `streamableHttp.ts:707`–`825`, directly contradicting the PR's stated approach of fixing types rather than call sites. The fixes are straightforward: add `| undefined` to `requestedTtl` in `CreateTaskServerContext` and `TaskServerContext` (interfaces.ts), add `| undefined` to all nested opt

Check failure on line 125 in packages/server/src/server/server.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

Server constructor conditional spread introduces unintended behavioral regression

The Server constructor uses an unnecessary spread pattern that changes semantics: when capabilities.tasks is not set, the old code explicitly passed tasks: undefined to Protocol (overriding any options.tasks from ProtocolOptions), while the new code leaves options.tasks intact, allowing a user-supplied task store to become active even though no task capability was advertised. The fix is to use the original tasks: extractTaskManagerOptions(...) pattern, now valid under exactOptionalPropertyTypes