fix(bootstrap): abort hung Cloudflare terminal handshake - #18
Conversation
The Cloudflare Crabbox terminal upgrade fetch had no AbortSignal. Post-connect diagnostics already time out at 15s, but a handshake that never returns HTTP 101 could hang the isolate. AWS already uses handshakeTimeout 15_000. Clear the timer after a successful upgrade so an accepted socket is not torn down. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
Codex review: needs real behavior proof before merge. Reviewed August 15, 2026, 10:42 PM ET / August 16, 2026, 02:42 UTC. ClawSweeper reviewWhat this changesThe PR adds a 15-second abort to the Cloudflare terminal WebSocket upgrade request, adds a focused timeout test, and adds an unreleased changelog entry. Regression provenancePossible regression — probable (reproduction; reviewed change). No predecessor PR is attributed. Merge readiness⛔ Blocked until stronger real behavior proof is added - 4 items remain Keep this PR open: current main still has an unbounded Cloudflare terminal upgrade, and the patch is a sound bounded repair. Before merge, remove the release-owned changelog edit and provide redacted Cloudflare Worker or staging proof of the actual upgraded-fetch path. Priority: P2 Review scores
Verification
How this fits togetherCrabhelm’s Cloudflare control plane opens a bearer-authenticated outbound WebSocket to a Crabbox workspace terminal for runtime diagnostics. The dialer returns an accepted socket to bounded diagnostic collection, or should return a bounded error when the terminal handshake does not complete. flowchart LR
A[Operator diagnostics request] --> B[Cloudflare control plane]
B --> C[Terminal dialer]
C --> D[Crabbox upgrade endpoint]
C --> E{Handshake within 15 seconds?}
E -->|Yes| F[Accepted terminal socket]
E -->|No| G[Bounded timeout error]
F --> H[Redacted diagnostics result]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep the scoped timeout implementation and regression test, remove the release-owned changelog line, and land only after a redacted Worker or staging run proves both the 15-second failure path and an unaffected successful upgrade. Do we have a high-confidence way to reproduce the issue? No high-confidence Cloudflare Worker reproduction is supplied. Current source clearly shows the unbounded fetch and the branch test establishes the intended abort path, but the supplied live experiment is Node rather than the changed Worker runtime. Is this the best way to solve the issue? Yes for the code path: a handshake-scoped AbortController with finally cleanup is the narrowest repair and preserves accepted sockets. The release-owned changelog edit should be removed, and Worker-runtime proof is still needed. Full review comments:
Overall correctness: patch is correct AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning high; reviewed against 71b80d86ca0b. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
History |
What Problem This Solves
Cloudflare workspace bootstrap upgrades the Crabbox terminal with
fetch()and no abort signal. After connect, diagnostics already time out at 15s. A handshake that accepts TCP and never returns HTTP 101 can hang the isolate forever. The AWS dialer already setshandshakeTimeout: 15_000.Evidence
Live Node against a TCP server that accepts the connection and never writes an HTTP response, then against the production
cloudflareTerminalDialerpath with a hungfetch.Patched call site on this branch:
A hung upgrade now fails in 15s with
Crabbox terminal handshake timed out. The timer is cleared after a successful 101 so an accepted socket is not torn down.Real behavior proof
fetch()now carries a 15s handshake abort. A hung attach URL finishes asCrabbox terminal handshake timed outinstead of leaving the Worker fetch pending./tmp/oc-impl-crabhelm-handshakeonfix/cf-terminal-handshake-timeout. Livenodeagainst a local hanging HTTP server (accept, no response body) and the production default terminal dialer.node:httpserver that never writes a response. Comparedfetchwith no signal (still pending at 250ms) toAbortSignal.timeout(80). Then calledCrabboxWorkspaceBootstrap.runtimeDiagnostics()with a hungfetchso the compiled 15s handshake budget is the one inworker/bootstrap.ts.fetchstayed pending. The 80ms helper aborted withTimeoutErrorat 82ms. Production runtime diagnostics recordedCrabbox terminal handshake timed outat 15001ms. The abort timer is cleared after a successful upgrade.wshandshakeTimeout (already set inaws/terminal-dialer.ts).Summary
handshakeTimeoutand the existing post-connect diagnostics timerfetchlanded in openclaw/crabhelm#8 (94d9d64, 2026-07-07) and has been present for 39 days