Skip to content

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

Merged
steipete merged 4 commits into
openclaw:mainfrom
Quack6765:agent/fix-idle-legacy-sse-startup
Aug 10, 2026
Merged

fix(transport): avoid stall when legacy SSE headers stay pending#282
steipete merged 4 commits into
openclaw:mainfrom
Quack6765:agent/fix-idle-legacy-sse-startup

Conversation

@Quack6765

@Quack6765 Quack6765 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

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 real behavior proof before merge. Reviewed August 9, 2026, 9:41 PM ET / August 10, 2026, 01:41 UTC.

ClawSweeper review

What this changes

Caps legacy standalone-SSE startup waiting at 250 ms so HTTP MCP list and call operations continue when the receive stream has not yet returned headers.

Regression provenance

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

Merge readiness

Blocked until stronger real behavior proof is added - 3 items remain

Keep open: current main still waits unboundedly for legacy standalone-SSE response headers, while this PR supplies a focused bounded-wait repair and regression coverage. The collaborator’s explicit request for an authenticated Mobbin run from the final head remains the merge gate.

Priority: P2
Reviewed head: fc53ee00d4958673e23326c9067f56f0298afb7a

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The implementation and focused coverage are strong, but proof of the affected authenticated provider has not been refreshed on the final head.
Proof confidence 🦐 gold shrimp (3/6) Needs stronger real behavior proof before merge: Fixture and earlier-head live evidence are strong, but the collaborator requires a redacted authenticated Mobbin result from final head fc53ee0 before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🦞 diamond lobster (5/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: Fixture and earlier-head live evidence are strong, but the collaborator requires a redacted authenticated Mobbin result from final head fc53ee0 before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 5 items Current-main stall path: The tracked fetch resolves its startup promise only after the standalone SSE fetch settles; a response that has not delivered headers leaves that promise unresolved.
Unbounded legacy wait: After a legacy connection succeeds, current main directly awaits the standalone-SSE startup promise before returning the usable client context.
Released behavior provenance: Blame attributes the current tracking and unbounded await to the v0.13.2 release commit, and that commit is contained by the v0.13.2 tag; the reported behavior is therefore still shipped.
Findings None None.
Security None None.

How this fits together

MCPorter’s HTTP transport connects configured MCP servers and then feeds the resulting client to CLI list and call operations. For legacy protocol servers, the SDK opens a long-lived SSE receive channel after initialization, and its startup timing affects both the first request and later server-initiated messages.

flowchart TD
  A[Configured HTTP MCP server] --> B[Legacy connection setup]
  B --> C[Standalone SSE receive request]
  C --> D{Headers arrive during grace?}
  D -->|Yes| E[Start list or call]
  D -->|No| F[Continue after bounded grace]
  F --> E
  C --> G[Long-lived server message stream]
  E --> H[CLI tool result]
  G --> H
Loading

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: Fixture and earlier-head live evidence are strong, but the collaborator requires a redacted authenticated Mobbin result from final head fc53ee0 before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Resolve merge risk (P1) - The 250 ms readiness bound changes legacy request ordering; fixtures cover delayed headers and elicitation, but final-head authenticated Mobbin evidence is still needed to confirm the intended hosted-server path.
  • Complete next step (P2) - A collaborator has already set a concrete final-head external-proof gate; no further mechanical code repair is currently indicated.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production versus regression coverage production +24/-3, tests +246/-0, release notes +4 The small transport change is backed by pending-header, delayed-header, and abort coverage rather than broad production churn.

Merge-risk options

Maintainer options:

  1. Capture final-head hosted-server proof (recommended)
    Run the built final candidate against Mobbin, redact private data, and attach the successful list result before merging.
  2. Pause pending provider validation
    Keep the PR open if final-head authenticated access is unavailable, because the reported provider path remains unverified.

Technical review

Best possible solution:

Publish a redacted final-head built-CLI Mobbin list result, preferably with one safe read-only call, then merge the narrow transport repair if the observed result remains successful.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level: a legacy server that accepts the standalone SSE GET but withholds headers leaves current main awaiting the tracked fetch before list or call. This read-only review did not execute that fixture.

Is this the best way to solve the issue?

Yes, the bounded grace preserves normal early readiness while preventing the proven unbounded wait, and the branch covers a later-header elicitation path; final-head provider proof is still required before merge.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against e404ed575a47.

Labels

Label justifications:

  • P2: This is a focused reliability regression for legacy hosted HTTP MCP servers, with limited blast radius but a real list/call failure.
  • merge-risk: 🚨 other: The patch changes legacy connection readiness ordering, and final-head validation against the affected authenticated provider is still outstanding.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦞 diamond lobster.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Fixture and earlier-head live evidence are strong, but the collaborator requires a redacted authenticated Mobbin result from final head fc53ee0 before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

  • Current-main stall path: The tracked fetch resolves its startup promise only after the standalone SSE fetch settles; a response that has not delivered headers leaves that promise unresolved. (src/runtime/http-transport.ts:99, e404ed575a47)
  • Unbounded legacy wait: After a legacy connection succeeds, current main directly awaits the standalone-SSE startup promise before returning the usable client context. (src/runtime/http-transport.ts:270, e404ed575a47)
  • Released behavior provenance: Blame attributes the current tracking and unbounded await to the v0.13.2 release commit, and that commit is contained by the v0.13.2 tag; the reported behavior is therefore still shipped. (src/runtime/http-transport.ts:274, a8b5a681b339)
  • Focused branch coverage: The final PR metadata shows a bounded grace in the transport plus pending-header, delayed-header elicitation, and abort-focused regression coverage; the prior released-section finding is resolved by placing the note under unreleased 0.13.3. (src/runtime/http-transport.ts:121, fc53ee00d495)
  • Outstanding real-provider gate: A collaborator verified fixture and CI evidence at the repair head but explicitly requested a redacted authenticated Mobbin list, preferably a safe call, from the final candidate before merge. (fc53ee00d495)

Likely related people:

  • Peter Steinberger: Current-main blame assigns the legacy SSE startup path to the release commit, and the related SDK v2 migration and final repair commits are attributed to Peter Steinberger. (role: recent transport and SDK-migration contributor; confidence: high; commits: a8b5a681b339, a6647f694dcd, 8fedfb057126; files: src/runtime/http-transport.ts)
  • umutkeltek: The merged standalone-SSE isolation work is the adjacent transport behavior whose byte-idle case this PR preserves separately. (role: adjacent SSE transport contributor; confidence: medium; commits: 57cf3b19aba6; files: src/runtime/http-transport.ts, tests/cli-idle-sse.integration.test.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Attach a redacted final-head built-CLI Mobbin list transcript with the candidate SHA and clean exit.
  • Preferably include one safe read-only Mobbin call, with private response data removed.

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 (42 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-09T16:20:47.387Z sha 8fedfb0 :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-09T17:32:24.929Z sha 8fedfb0 :: needs real behavior proof before merge. :: [P2] Remove the stale released-version changelog entry
  • reviewed 2026-08-09T18:52:18.572Z sha 8fedfb0 :: needs changes before merge. :: [P2] Remove the stale release entry
  • reviewed 2026-08-09T19:29:33.896Z sha 8fedfb0 :: found issues before merge. :: [P2] Remove the stale release entry
  • reviewed 2026-08-09T21:04:14.579Z sha 8fedfb0 :: needs changes before merge. :: [P2] Remove the released changelog entry
  • reviewed 2026-08-09T21:12:53.465Z sha 8fedfb0 :: needs real behavior proof before merge. :: [P2] Remove the post-release changelog entry
  • reviewed 2026-08-09T22:54:37.695Z sha 8fedfb0 :: needs real behavior proof before merge. :: [P2] Remove the entry from the released 0.13.1 section
  • reviewed 2026-08-09T23:10:21.158Z sha 8fedfb0 :: needs real behavior proof before merge. :: [P2] Remove the entry from the released 0.13.1 section

@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
Contributor 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. 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. 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. 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. labels Aug 6, 2026
@Quack6765
Quack6765 marked this pull request as ready for review August 7, 2026 10:59
@clawsweeper clawsweeper Bot added 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. and removed 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. labels Aug 7, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. 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. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Aug 9, 2026
@steipete
steipete merged commit 506b030 into openclaw:main Aug 10, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants