fix(codex): keep websocket sessions alive - #4639
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1be39487e8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| case <-c.done: | ||
| return | ||
| case <-ticker.C: | ||
| errWrite := c.conn.WriteControl(websocket.PingMessage, nil, time.Now().Add(writeTimeout)) |
There was a problem hiding this comment.
Don't let heartbeat pings time out behind data writes
When a request or tool result is being written for longer than 10s, such as a large payload over a slow proxy, this heartbeat WriteControl can wait behind the in-progress data write until the supplied deadline expires; the error path then closes and invalidates an otherwise healthy session. Since the heartbeat is meant to protect idle connections, please skip/serialize pings during application writes or avoid a finite post-connect write deadline so active uploads are not aborted.
AGENTS.md reference: AGENTS.md:L58-L58
Useful? React with 👍 / 👎.
Summary
Motivation
Codex upstream WebSocket sessions can be quiet between turns. HTTP/SOCKS relays and other intermediaries may reap those idle connections before CPA's five-minute read deadline. The next request then surfaces a
broken pipe,unexpected EOF, or abnormal WebSocket close even though the original upgrade succeeded.The existing code responds to server Ping frames but does not proactively send Ping frames, and Pong frames do not extend the current read deadline.
Validation
go test -race ./internal/runtime/executor -run TestCodexWebsocketHeartbeat -count=3go test ./...go build -o /tmp/cliproxyapi-ws-heartbeat-build ./cmd/server7.2.102: the reused upstream connection closed during idle after about 199 seconds with WebSocket1006 unexpected EOFsession_closedNo configuration or public API changes are required.