Rebuild the streamable HTTP server transport around per-request dispatch - #3192
Open
maxisbey wants to merge 8 commits into
Open
Rebuild the streamable HTTP server transport around per-request dispatch#3192maxisbey wants to merge 8 commits into
maxisbey wants to merge 8 commits into
Claude / Claude Code Review
completed
Jul 28, 2026 in 40m 47s
Code review found 3 potential issues
Found 3 candidates, confirmed 3. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 3 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | docs/migration.md:923-926 |
migration.md last behaviour bullet attributes CONNECTION_CLOSED to the JSON-mode request-scoped case, contradicting bull |
| 🟡 Nit | docs/migration.md:866 |
migration.md claims transport rework "does not touch the public surface" while the next section documents the public con |
Annotations
Check warning on line 926 in docs/migration.md
claude / Claude Code Review
migration.md last behaviour bullet attributes CONNECTION_CLOSED to the JSON-mode request-scoped case, contradicting bullet 1 and the code (NoBackChannelError/INVALID_REQUEST)
The last bullet of the "Behaviour clarified in the same change" list attributes `CONNECTION_CLOSED` to "a request-scoped [server-to-client request] in JSON-response mode", but that case never reaches the `CONNECTION_CLOSED` path — it raises `NoBackChannelError` (serialized as `INVALID_REQUEST`), exactly as the first bullet of the same list already documents. Deleting the ", or a request-scoped one in JSON-response mode" parenthetical resolves the self-contradiction; `CONNECTION_CLOSED` is accura
Check warning on line 866 in docs/migration.md
claude / Claude Code Review
migration.md claims transport rework "does not touch the public surface" while the next section documents the public connect() removal
The parenthetical at docs/migration.md:866 says the transport rework "does not touch the public surface", but the very next section (added by this same PR) documents that the rework removes the public `StreamableHTTPServerTransport.connect()` context manager and adds keyword-only `app`/`lifespan_state` constructor arguments — the PR description itself calls `connect()` removal "the one API removal". Consider rewording to something like "(whose only public-surface change is the removal of `transp
Loading