Repository navigation
fix(sdk): send WebSocket keepalive pings on PTY sessions - #319
Merged
Merged
Conversation
Vidoc security reviewTip Good to merge — no security issues found. Reviewed 11 changed files. 💬 Have questions? Tag @vidoc in a comment and I'll answer. |
There was a problem hiding this comment.
All reported issues were addressed across 11 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
A PTY that stays silent for a long time (a build or test run producing no output for tens of minutes) carries no traffic, and intermediate proxies/load balancers close idle WebSockets. The shell keeps running inside the sandbox but the client loses the stream and must reconnect; output produced in between is not replayed. Send a ping frame every 20s from every SDK so the connection never looks idle: - python: explicit httpx-ws keepalive on sync PTY connects; aiohttp heartbeat on the shared async WebSocket opener - typescript: setInterval(ws.ping) on runtimes whose socket supports it (Node ws); browsers cannot send pings and are skipped - go: keepalive goroutine using WriteControl(PingMessage) - java: OkHttp pingInterval on the PTY client - ruby: keepalive thread sending :ping frames, serialized with input writes Signed-off-by: MDzaja <mirkodzaja0@gmail.com>
MDzaja
force-pushed
the
fix/pty-websocket-keepalive
branch
from
October 5, 2026 13:44
d6aa71a to
2b03fcf
Compare
Collaborator
Author
|
@cubic-dev-ai review |
@MDzaja I have started the AI code review. It will take a few minutes to complete. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A PTY session can legitimately stay silent for a long time — a build or test run that produces no output for 20–60 minutes. During that window the WebSocket carries no traffic at all, and intermediate proxies / load balancers between the client and the sandbox close connections they consider idle. The shell keeps running inside the sandbox, but the client loses the stream and has to reconnect with
connect_pty_session; output produced between the drop and the reconnect is not replayed.Before this change only the sync Python SDK sent pings (via the
httpx-wslibrary default). Every other SDK sent nothing on an idle PTY.Change
Every SDK now sends a WebSocket ping frame every 20 s on PTY sessions. Ping/pong are control frames: they keep the connection active for every hop on the path without injecting any input into the terminal.
keepalive_ping_interval_seconds/keepalive_ping_timeout_secondson the two PTYconnect_wscalls (makes the previous library default explicit)heartbeat=on the shared_open_wsopener (also covers log streaming sockets, which have the same idle problem)setInterval(ws.ping)when the socket exposesping()(Nodews); cleared on close/error,unref'd. BrowserWebSocketcannot send pings, so it is skipped thereWriteControl(PingMessage)(safe alongsideWriteMessage), stops when the session endsclient.newBuilder().pingInterval(20s)for the PTY socket (shares pool/dispatcher with the base client):pingframes; input writes and pings are serialized with a mutex; stopped on close/disconnectThe interval (20 s) sits comfortably below common LB idle timeouts (60 s+) and matches the pre-existing
httpx-wsdefault.Tests
tests/test_process_keepalive.pyasserts keepalive kwargs on syncconnect_wsandheartbeaton asyncws_connectping()fires every 20 s while open, stops after close, and is never scheduled on sockets withoutping()TestPtyHandleSendsKeepalivePings— httptest ws server counts pings, verifies they stop afterDisconnect()(-raceclean)disconnectPtyHandleTestpasses against the ping-enabled clientVerified per SDK: pytest + pylint + basedpyright, jest + eslint + prettier,
go test -race+ golangci-lint, rspec + rubocop, gradle test.Summary by cubic
Prevents idle PTY sessions from being dropped by proxies and load balancers by sending WebSocket ping frames every 20 seconds from every SDK. Previously only the sync Python SDK sent pings; the rest sent nothing during long silent runs, so clients lost the stream and output produced between the drop and reconnect was not replayed. The interval sits below common LB idle timeouts and matches the pre-existing
httpx-wsdefault.keepalive_ping_interval_seconds/keepalive_ping_timeout_secondson PTYconnect_wscallsheartbeat=on the shared_open_wsopener, which also covers log streaming socketssetInterval(ws.ping)on Nodews; skipped in browsers, which cannot send pingsWriteControl(PingMessage), stopped when the session endspingIntervalon the PTY socket:pingframes, serialized with input writesTests
go test -race, rspec, and existing gradle tests.Written for commit 2b03fcf. Summary will update on new commits.
Verified against a live environment
Python SDK from this branch, three PTY sessions in parallel, each running
sleep 1500; echo DONE; exit(25 minutes with zero output):httpx-ws)DONEreceived after 1530 s, exit code 0aiohttp heartbeat=20)DONEreceived after 1531 s, exit code 0heartbeat=None(pre-PR behaviour)DONEor a close frame:is_connected()stayedTrueandwait()hung indefinitelyThe control case shows the pre-PR failure mode is worse than a clean drop: an intermediate hop silently discards the idle connection and the client is left with a half-open socket. With pings, the path stays active, and if it does die a missed pong surfaces as a close within the ping interval instead of a hang.