fix(transport): avoid stall when legacy SSE headers stay pending - #282
fix(transport): avoid stall when legacy SSE headers stay pending#282Quack6765 wants to merge 2 commits into
Conversation
|
Codex review: needs maintainer review before merge. Reviewed August 6, 2026, 11:34 AM ET / 15:34 UTC. ClawSweeper reviewWhat this changesThe PR caps the legacy standalone SSE startup wait so tool requests continue when the server delays SSE response headers. Regression provenancePossible regression — probable (reproduction; reviewed change). No predecessor PR is attributed. Merge readinessKeep open: current main still has the unbounded legacy SSE readiness wait, and this PR now addresses the prior ordering concern with delayed-header elicitation coverage. No blocking patch defect was found. Priority: P2 Review scores
Verification
How this fits togetherMCPorter’s HTTP transport connects CLI list/call requests to remote MCP servers. Legacy servers also open a separate SSE receive stream for server-initiated traffic, whose startup ordering affects the first client request. flowchart LR
A[CLI list or call] --> B[HTTP transport]
B --> C[Legacy MCP connection]
C --> D[Standalone SSE receive stream]
D --> E[Header readiness wait]
E --> F[Tool request proceeds]
D --> G[Server initiated messages]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Retain the bounded legacy wait and delayed-header elicitation coverage, then land after the draft is made ready and ordinary required checks are satisfied. Do we have a high-confidence way to reproduce the issue? Yes—current main’s legacy path awaits the standalone SSE fetch without a bound, and the submitted pending-header fixture gives a focused current-main failure path; this read-only review did not execute it. Is this the best way to solve the issue? Yes—the bounded wait keeps normal fast-header ordering while avoiding an unbounded pre-header stall, and the follow-up compatibility test directly covers the earlier ordering concern. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 4219927ec680. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
History |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
What Problem This Solves
MCPorter 0.13.0 can finish OAuth successfully and still report a legacy HTTP MCP server as offline. After
notifications/initialized, the SDK v2 client starts a long-lived standalone SSEGET; MCPorter waits for that fetch promise before allowinglistorcallto continue. A fetch promise does not settle until response headers arrive, so a server that accepts the SSE request but leaves its response completely idle can hold MCPorter at startup until the outer command timeout.That is the live Mobbin failure behind this PR: authentication completes, but
mcporter list mobbintimes out after 30 seconds. This is distinct from #234, which isolates a byte-idle SSE response that has already sent headers from the ordinary request connection pool. The unbounded pre-header wait was added with the SDK v2 migration in #255.Why This Change Was Made
The repair is deliberately narrow. Legacy connections still give the standalone SSE receive channel a chance to establish, but that readiness wait is capped at 250 ms:
Removing the readiness wait entirely made the existing legacy HTTP elicitation/sampling fixture race its first tool call ahead of the receive channel. Keeping the wait unbounded reproduces the reported stall. The bounded grace passes both paths without adding configuration, dependencies, or changes to OAuth behavior.
The pending-header regression fixture accepts the standalone SSE request without flushing response headers, matching the observed hosted-server behavior. A second compatibility test routes a real legacy fixture through a proxy that forwards the SSE request immediately but withholds its response headers for 500 ms—twice the startup grace. Its first tool call begins after the grace, receives a later
elicitation/createrequest on the still-open stream, returns the headless decline, and completes successfully. This directly covers the delayed-header ordering boundary raised by ClawSweeper.The production change remains confined to
src/runtime/http-transport.ts; the other changes are focused integration coverage and the 0.13.1 changelog entry.User Impact
Legacy hosted MCP servers such as Mobbin can list and call tools even when their standalone SSE response does not immediately send headers. Servers that establish the receive channel normally retain the existing readiness ordering. When headers arrive after the grace, the receive stream remains active and can still deliver server-initiated elicitation, as verified with a 500 ms delay. Header-idle servers incur at most the 250 ms startup grace per new connection instead of hanging until the command timeout.
No configuration, credentials, OAuth persistence, protocol selection, or public API changes.
Evidence
Exact PR head
8412b06, macOS, Node 24.6.0, pnpm 10.33.2.Live authenticated Mobbin run from the built PR head (redacted transcript; no token or credential output):
Non-vacuous regression proof: with only the production await reset to current
main's unboundedawait transportOptions.standaloneSseStarted, the new pending-header fixture fails as offline at its 2-second command timeout:Restoring the bounded wait makes the same fixture pass:
Delayed-header compatibility proof requested by ClawSweeper: the proxy withholds standalone SSE response headers for 500 ms, beyond the 250 ms grace, while the first legacy call completes a server-initiated elicitation exchange:
Repository gates:
Scope: four files,
+93 / -5; no dependency, configuration, or public API changes.