fix(tlon): clear SSE connect timeout after failed openStream by hugenshen · Pull Request #104585 · openclaw/openclaw

hugenshen · 2026-07-11T17:19:11Z

What Problem This Solves

Fixes an issue where Tlon Urbit SSE openStream armed a 60s connect setTimeout and only cleared it after a successful urbitFetch. When the connect fetch rejected (DNS/SSRF/network) or returned non-OK, the timer stayed armed until it fired, and reconnect storms could stack dangling timers in the Gateway process.

Why This Change Was Made

Wrap the connect fetch in try/finally and always clearTimeout, matching the zalouser probe timeout cleanup pattern, so failed connects release the timer immediately without changing the connect deadline or stream lifecycle.

User Impact

Flaky Urbit hosts no longer leave dangling connect timers in long-running Tlon monitors during reconnect failures.

Evidence

Unit regression (mocked fetch + fake timers)

node scripts/run-vitest.mjs extensions/tlon/src/urbit/sse-client.test.ts --reporter=verbose

 ✓ ... > openStream > clears the connect timeout when urbitFetch rejects 12ms
 ✓ ... > openStream > clears the connect timeout when the stream response is not ok 10ms
 Test Files  1 passed (1)
      Tests  18 passed (18)

Production-path connect-timeout storm proof

node scripts/run-vitest.mjs extensions/tlon/src/urbit/sse-client.open-stream-timeout.proof.test.ts --reporter=verbose

 ✓ ... > clears connect timers after a real non-OK stream response storm 305ms
 ✓ ... > clears the connect timer when production urbitFetch rejects 10ms
 Test Files  1 passed (1)
      Tests  2 passed (2)

The storm case runs production openStream → urbitFetch → fetchWithSsrFGuard against a real loopback node:http server that returns 503 five times; real timers; each armed setTimeout(..., 60000) is cleared. The reject case uses a closed loopback port (127.0.0.1:9) so the production transport fails before headers.

Real behavior proof

Behavior or issue addressed: Failed SSE connect attempts no longer leave uncleared 60s AbortController timers, including under repeated failure.
Canonical reachability path: Tlon monitor reconnect → UrbitSSEClient.openStream → production urbitFetch / fetchWithSsrFGuard → reject or non-OK → finally { clearTimeout(timeoutId) }.
Boundary crossed: local-runtime HTTP; loopback node:http + closed-port TCP failure; production SSRF-guarded transport (no urbitFetch mock, no fake timers).
Shared helper / provider constraint check: Same try/finally { clearTimeout } pattern as extensions/zalouser/src/probe.ts; no new config/API.
Real environment tested: macOS, Node via node scripts/run-vitest.mjs with real HTTP listeners and real timers.
Exact steps or command run after this patch: node scripts/run-vitest.mjs extensions/tlon/src/urbit/sse-client.open-stream-timeout.proof.test.ts extensions/tlon/src/urbit/sse-client.test.ts --reporter=verbose
Evidence after fix:

 ✓ |extension-messaging| ... > clears the connect timeout when urbitFetch rejects 12ms
 ✓ |extension-messaging| ... > clears the connect timeout when the stream response is not ok 10ms
 ✓ |extension-messaging| ... > clears connect timers after a real non-OK stream response storm 305ms
 ✓ |extension-messaging| ... > clears the connect timer when production urbitFetch rejects 10ms
 Test Files  2 passed (2)
      Tests  20 passed (20)
[test] passed 1 Vitest shard in 7.90s

Observed result after fix: Five sequential non-OK connects each arm then clear a 60s connect timer with matching clearTimeout(handle); a real transport reject also clears its single connect timer.
What was not tested: Live Urbit ship reconnect storm against a real Tlon host; SSE stream body processing after a successful connect.
Fix classification: Root cause fix.

Risk / Compatibility

Low. Successful connects still clear the timer after headers; only failed connects gain the missing cleanup.

clawsweeper · 2026-07-11T17:31:31Z

Codex review: needs maintainer review before merge. Reviewed July 14, 2026, 9:25 PM ET / July 15, 2026, 01:25 UTC.

Summary
The PR always clears the Tlon SSE connection timer after openStream fetch completion or failure and adds mocked plus real loopback-network regression coverage.

PR surface: Source -1, Tests +159. Total +158 across 3 files.

Reproducibility: yes. at the source level: current main leaves the timer armed when urbitFetch rejects, and the PR supplies focused production-path proof for both rejection and non-OK responses. I did not independently execute a current-main failing run during this read-only review.

Review metrics: none identified.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

[P1] A live authenticated Urbit ship reconnect storm was not exercised, although the submitted loopback HTTP and closed-port proof crosses the relevant production transport boundary.

Maintainer options:

Decide the mitigation before merge
Land the try/finally cleanup with both regression suites intact, provided the clean three-way merge result and required checks remain green on the exact head.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

No automated repair is indicated; the patch has no actionable finding and only needs ordinary maintainer landing review.

Security
Cleared: The localized runtime and test changes introduce no dependency, secret, permission, downloaded-code, or supply-chain surface and preserve the existing SSRF-guarded transport.

Review details

Best possible solution:

Land the try/finally cleanup with both regression suites intact, provided the clean three-way merge result and required checks remain green on the exact head.

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

Yes at the source level: current main leaves the timer armed when urbitFetch rejects, and the PR supplies focused production-path proof for both rejection and non-OK responses. I did not independently execute a current-main failing run during this read-only review.

Is this the best way to solve the issue?

Yes. Moving the existing timer cleanup into finally is the narrowest maintainable fix because it covers every fetch exit path without changing the timeout deadline, reconnect policy, or successful stream lifecycle.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: The PR fixes bounded resource retention during repeated Tlon connection failures without changing configuration, APIs, or normal successful-stream behavior.
rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): After-fix live output demonstrates repeated real HTTP 503 responses and a real TCP connection rejection through the production SSRF-guarded transport, with every 60-second timer handle cleared.
proof: sufficient: Contributor real behavior proof is sufficient. After-fix live output demonstrates repeated real HTTP 503 responses and a real TCP connection rejection through the production SSRF-guarded transport, with every 60-second timer handle cleared.

Evidence reviewed

PR surface:

Source -1, Tests +159. Total +158 across 3 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	31	32	-1
Tests	2	159	0	+159
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	3	190	32	+158

What I checked:

Current-main defect: The current implementation clears the 60-second connection timer only after urbitFetch resolves, leaving it armed when the fetch rejects. (extensions/tlon/src/urbit/sse-client.ts:188, 3c0654c00271)
Root-cause repair: The proposed try/finally preserves the successful-stream lifecycle while clearing the connection timer for rejected and non-OK connection attempts. (extensions/tlon/src/urbit/sse-client.ts:188, 2f3769f12fac)
Focused regression coverage: The modified unit suite covers both rejected transport and non-OK response paths, including release cleanup and zero remaining fake timers. (extensions/tlon/src/urbit/sse-client.test.ts:120, 2f3769f12fac)
Production-path proof: The added proof exercises production openStream, urbitFetch, and SSRF-guarded transport against a real loopback HTTP server and a closed TCP port using real timers. (extensions/tlon/src/urbit/sse-client.open-stream-timeout.proof.test.ts:1, 2f3769f12fac)
Feature provenance: Merged pull request fix(tlon): add timeout to SSE client fetch calls (CWE-400) #5926 introduced the SSE connection timer; its review discussion explicitly identified missing cleanup when the fetch rejects or aborts. (extensions/tlon/src/urbit/sse-client.ts:188, 411d5fda587c)
Review continuity: The current head matches the previously reviewed SHA, all earlier ClawSweeper cycles reported no findings, and exact-head CI plus the real-behavior proof checks succeeded. (2f3769f12fac)

Likely related people:

hclsys: Authored the merged change that added the 60-second SSE connection timer and established the cleanup behavior this PR repairs. (role: introduced timeout behavior; confidence: high; commits: 6b5460c3d18f, 411d5fda587c; files: extensions/tlon/src/urbit/sse-client.ts)
Pick-cat: Recently carried substantial merged hardening work in the same Tlon SSE client and its regression suite. (role: recent area contributor; confidence: high; commits: c895c97d66d0, 1710ccad2977; files: extensions/tlon/src/urbit/sse-client.ts, extensions/tlon/src/urbit/sse-client.test.ts)
steipete: Merged and integrated the original Tlon timeout change, making this a useful routing candidate for the narrow follow-up. (role: merger and recent integrator; confidence: medium; commits: 411d5fda587c, 13c8b2a21c1a; files: extensions/tlon/src/urbit/sse-client.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works

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.

Review history (28 earlier review cycles; latest 8 shown)

reviewed 2026-07-14T22:31:43.549Z sha 2f3769f :: needs maintainer review before merge. :: none
reviewed 2026-07-14T22:49:59.129Z sha 2f3769f :: needs maintainer review before merge. :: none
reviewed 2026-07-14T23:06:55.584Z sha 2f3769f :: needs maintainer review before merge. :: none
reviewed 2026-07-14T23:24:42.855Z sha 2f3769f :: needs maintainer review before merge. :: none
reviewed 2026-07-14T23:42:38.090Z sha 2f3769f :: needs maintainer review before merge. :: none
reviewed 2026-07-14T23:59:18.425Z sha 2f3769f :: needs maintainer review before merge. :: none
reviewed 2026-07-15T00:17:27.530Z sha 2f3769f :: needs maintainer review before merge. :: none
reviewed 2026-07-15T00:40:35.487Z sha 2f3769f :: needs maintainer review before merge. :: none

hugenshen · 2026-07-11T18:20:55Z

@clawsweeper re-review

clawsweeper · 2026-07-11T18:20:58Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

State: Complete
Detail: The targeted re-review published an exact durable review comment and queued an executing target-wide serialized router scan.
Run: https://github.com/openclaw/clawsweeper/actions/runs/29163440972
Updated: 2026-07-11T18:34:16.993Z

steipete · 2026-07-15T23:32:37Z

Land-ready proof for exact head fbcb0124b88a833c1b69a0ee655c32b68a869fee:

Refactored the cleanup boundary so the 60-second deadline owns only the response-header wait. Rejected fetches always disarm it; non-OK responses clear it before releasing the guarded response; successful SSE body ownership is unchanged.
Testbox tbx_01kxm147at2gxpgxffdspz25z2 (run 29458087968): pnpm test extensions/tlon/src/urbit/sse-client.test.ts extensions/tlon/src/urbit/sse-client.open-stream-timeout.proof.test.ts -- --reporter=verbose — 26/26 passed.
That proof includes five sequential real loopback HTTP 503 connections and a real socket rejection through openStream -> urbitFetch -> fetchWithSsrFGuard; every armed connect timer was cleared.
Same Testbox: touched-file oxfmt --check and pnpm check:test-types passed.
Fresh Codex autoreview: no accepted/actionable findings (0.91 confidence).
Hosted exact-head CI is green: run 29458383743.
Repo-native OPENCLAW_TESTBOX=1 scripts/pr prepare-run 104585 passed and verified the hosted head.

Known gap: no live reconnect storm was induced against a real Tlon ship. The exercised network path is the production SSRF-guarded transport with real sockets and timers; no fetch mock or fake timer is used in the integration proof.

steipete · 2026-07-15T23:32:58Z

Merged via squash.

Prepared head SHA: fbcb0124b88a833c1b69a0ee655c32b68a869fee
Landed commit: b18f1a8ecda188f9440b8b2f1791a84179dd3560

…w#104585) * fix(tlon): clear SSE connect timeout after failed openStream * refactor(tlon): scope SSE connect timeout --------- Co-authored-by: Peter Steinberger <[email protected]>

openclaw-barnacle Bot added channel: tlon Channel integration: tlon size: S labels Jul 11, 2026

hugenshen force-pushed the fix/tlon-sse-connect-timeout-cleanup branch from 1c618ac to a6f2fb7 Compare July 11, 2026 18:19

openclaw-barnacle Bot added size: M and removed size: S labels Jul 11, 2026

hugenshen force-pushed the fix/tlon-sse-connect-timeout-cleanup branch from a6f2fb7 to f79d2ee Compare July 12, 2026 00:07

clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 12, 2026

clawsweeper Bot mentioned this pull request Jul 12, 2026

fix(tlon): cap SSE payload JSON.parse at 16 MiB to prevent OOM #101274

Merged

hugenshen force-pushed the fix/tlon-sse-connect-timeout-cleanup branch from f79d2ee to 8bdd6d4 Compare July 13, 2026 15:06

hugenshen force-pushed the fix/tlon-sse-connect-timeout-cleanup branch 2 times, most recently from c597d5c to 2e5a551 Compare July 14, 2026 00:52

clawsweeper Bot removed the rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. label Jul 14, 2026

hugenshen force-pushed the fix/tlon-sse-connect-timeout-cleanup branch 2 times, most recently from 3ee883a to 5cf6f37 Compare July 14, 2026 14:04

fix(tlon): clear SSE connect timeout after failed openStream

2f3769f

hugenshen force-pushed the fix/tlon-sse-connect-timeout-cleanup branch from 5cf6f37 to 2f3769f Compare July 14, 2026 14:50

clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 14, 2026

steipete self-assigned this Jul 15, 2026

refactor(tlon): scope SSE connect timeout

fbcb012

steipete merged commit b18f1a8 into openclaw:main Jul 15, 2026
84 checks passed

github-actions Bot mentioned this pull request Jul 16, 2026

📡 Upstream Digest — 2026-07-16 01:51 UTC curtismercier/openclaw-mods#1308

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(tlon): clear SSE connect timeout after failed openStream#104585

fix(tlon): clear SSE connect timeout after failed openStream#104585
steipete merged 2 commits into
openclaw:mainfrom
hugenshen:fix/tlon-sse-connect-timeout-cleanup

hugenshen commented Jul 11, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 11, 2026 •

edited

Loading

Uh oh!

hugenshen commented Jul 11, 2026

Uh oh!

clawsweeper Bot commented Jul 11, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 15, 2026

Uh oh!

Uh oh!

steipete commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

hugenshen commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Unit regression (mocked fetch + fake timers)

Production-path connect-timeout storm proof

Real behavior proof

Risk / Compatibility

Uh oh!

clawsweeper Bot commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hugenshen commented Jul 11, 2026

Uh oh!

clawsweeper Bot commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented Jul 15, 2026

Uh oh!

Uh oh!

steipete commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hugenshen commented Jul 11, 2026 •

edited

Loading

clawsweeper Bot commented Jul 11, 2026 •

edited

Loading

clawsweeper Bot commented Jul 11, 2026 •

edited

Loading