Skip to content

fix(transport): avoid stall when legacy SSE headers stay pending - #282

Draft
Quack6765 wants to merge 2 commits into
openclaw:mainfrom
Quack6765:agent/fix-idle-legacy-sse-startup
Draft

fix(transport): avoid stall when legacy SSE headers stay pending#282
Quack6765 wants to merge 2 commits into
openclaw:mainfrom
Quack6765:agent/fix-idle-legacy-sse-startup

Conversation

@Quack6765

@Quack6765 Quack6765 commented Aug 6, 2026

Copy link
Copy Markdown

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 SSE GET; MCPorter waits for that fetch promise before allowing list or call to 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 mobbin times 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:

  • if response headers arrive sooner, MCPorter continues immediately;
  • if headers remain pending, MCPorter continues after 250 ms;
  • the SSE request is not cancelled and remains open in the background for later server-to-client messages.

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/create request 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):

$ git rev-parse --short HEAD
8412b06
$ node dist/cli.js list mobbin --brief
mobbin

  function search_screens(...): object;
  function search_flows(...): object;
  function search_sections(...): object;

  3 tools · 1147ms · HTTP https://api.mobbin.com/mcp

Non-vacuous regression proof: with only the production await reset to current main's unbounded await transportOptions.standaloneSseStarted, the new pending-header fixture fails as offline at its 2-second command timeout:

$ pnpm exec vitest run tests/cli-idle-sse.integration.test.ts --reporter=verbose
× lists tools while a standalone SSE response leaves its headers pending 2124ms

"status": "offline",
"durationMs": 2002,
"rawMessage": "Timeout"

Test Files  1 failed (1)
Tests       1 failed (1)

Restoring the bounded wait makes the same fixture pass:

$ pnpm exec vitest run tests/cli-idle-sse.integration.test.ts --reporter=verbose
✓ lists tools while a standalone SSE response leaves its headers pending 395ms

Test Files  1 passed (1)
Tests       1 passed (1)

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:

$ pnpm exec vitest run tests/e2e-fixture-servers.test.ts --reporter=verbose --testTimeout=30000 -t "headers arrive after the startup grace"
✓ handles legacy elicitation when standalone SSE headers arrive after the startup grace 651ms

Test Files  1 passed (1)
Tests       1 passed | 14 skipped (15)

Repository gates:

$ pnpm check
# format, OXLint, and TypeScript checks passed

$ pnpm test
Test Files  177 passed | 4 skipped
Tests       1,282 passed | 25 skipped

$ pnpm test:coverage
# passed every configured threshold
# statements 91.58% · branches 84.64% · functions 92.81% · lines 91.78%

Scope: four files, +93 / -5; no dependency, configuration, or public API changes.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. labels Aug 6, 2026
@clawsweeper

clawsweeper Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed August 6, 2026, 11:34 AM ET / 15:34 UTC.

ClawSweeper review

What this changes

The PR caps the legacy standalone SSE startup wait so tool requests continue when the server delays SSE response headers.

Regression provenance

Possible regression — probable (reproduction; reviewed change). No predecessor PR is attributed.

Merge readiness

⚠️ Ready for maintainer review - 2 items remain

Keep 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
Reviewed head: 8412b06985723643e3f18cba4a3e10ba4c9a7c7b

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The focused transport change and real behavior proof are strong, with the intentional legacy-ordering tradeoff covered by a targeted delayed-header test.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (terminal): The PR body supplies a redacted authenticated Mobbin CLI run plus after-fix pending-header and delayed-header compatibility transcripts on the stated head.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The PR body supplies a redacted authenticated Mobbin CLI run plus after-fix pending-header and delayed-header compatibility transcripts on the stated head.
Evidence reviewed 6 items Current-main behavior: Current main awaits the legacy standalone SSE promise with no timeout before returning the connected transport.
Bounded repair: The PR adds a 250 ms grace helper and uses it only for the legacy standalone SSE readiness wait.
Prior finding covered: The follow-up commit adds a real-CLI legacy elicitation case through a proxy that delays SSE headers for 500 ms, beyond the 250 ms grace.
Findings None None.
Security None None.

How this fits together

MCPorter’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]
Loading

Before merge

  • Resolve merge risk (P1) - The 250 ms cap deliberately relaxes the former readiness ordering for legacy servers whose SSE headers arrive later; the new 500 ms elicitation case reduces this compatibility risk but cannot exhaust every legacy ordering pattern.
  • Complete next step (P2) - No repair lane is needed: the prior code-level blocker is resolved, leaving normal draft and merge review workflow.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Diff scope 4 files; runtime +14/-3, tests +75/-2, release notes +4 Regression coverage materially outweighs the small production transport change.
Ordering boundary 250 ms grace; 500 ms delayed-header elicitation case The test exercises a header delay twice the production grace while preserving server-initiated legacy traffic.

Merge-risk options

Maintainer options:

  1. Accept the bounded ordering change (recommended)
    Land the 250 ms grace with the added 500 ms delayed-header elicitation coverage, accepting the narrow legacy-ordering tradeoff to avoid command-wide startup timeouts.

Technical review

Best 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.

Labels

Label changes:

  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body supplies a redacted authenticated Mobbin CLI run plus after-fix pending-header and delayed-header compatibility transcripts on the stated head.
  • remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

  • P2: A legacy HTTP server can make list/call wait until the command timeout, but the failure is bounded to that compatibility path.
  • merge-risk: 🚨 compatibility: The change intentionally permits the first legacy request before a slow SSE response has delivered headers.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body supplies a redacted authenticated Mobbin CLI run plus after-fix pending-header and delayed-header compatibility transcripts on the stated head.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies a redacted authenticated Mobbin CLI run plus after-fix pending-header and delayed-header compatibility transcripts on the stated head.

Evidence

What I checked:

Likely related people:

  • steipete: The related merged SDK v2 migration is the cited provenance for the legacy readiness behavior; current-line history is attributed to Peter Steinberger. (role: SDK v2 transport feature owner; confidence: high; commits: a6647f694dcd, 49dcd3e7fffd; files: src/runtime/http-transport.ts)
  • umutkeltek: The earlier idle-SSE transport fix established the neighboring isolated-stream behavior and regression fixture. (role: adjacent SSE transport contributor; confidence: medium; commits: 57cf3b19aba6; files: src/runtime/http-transport.ts, tests/cli-idle-sse.integration.test.ts)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (2 earlier review cycles)
  • reviewed 2026-08-06T15:11:22.710Z sha fce8a01 :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-06T15:19:32.439Z sha fce8a01 :: found issues before merge. :: [P2] Preserve readiness ordering for slow SSE headers

@Quack6765 Quack6765 changed the title fix(transport): bound legacy SSE startup wait fix(transport): avoid stall when legacy SSE headers stay pending Aug 6, 2026
@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 6, 2026
@Quack6765

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant