Skip to content

core: make EnvClient WebSocket keepalive configurable#877

Closed
sergiopaniego wants to merge 1 commit into
mainfrom
feature/ws-ping-config
Closed

core: make EnvClient WebSocket keepalive configurable#877
sergiopaniego wants to merge 1 commit into
mainfrom
feature/ws-ping-config

Conversation

@sergiopaniego

@sergiopaniego sergiopaniego commented Jun 29, 2026

Copy link
Copy Markdown
Member

Summary

Adds ping_interval_s / ping_timeout_s to EnvClient, forwarded to the WebSocket ws_connect. Default is 20.0 (the websockets default, so existing behavior is unchanged); passing ping_interval_s=None disables client-side keepalive, which sandbox tunnels that drop WebSocket ping/pong frames require to stay connected. Surfaced in #821 (Modal), where the E2E previously needed a monkeypatch of ws_connect.

Closes #875.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • New environment
  • Refactoring

Alignment Checklist

Before submitting, verify:

  • I have read .claude/docs/PRINCIPLES.md and this PR aligns with our principles
  • I have checked .claude/docs/INVARIANTS.md and no invariants are violated
  • I have run /pre-submit-pr (or bash .claude/hooks/lint.sh and tests) and addressed all issues

RFC Status

  • Not required (bug fix, docs, minor refactoring)
  • RFC exists: #___
  • RFC needed (will create before merge)

Test Plan

New unit tests (no network) patch ws_connect and assert the keepalive args are forwarded:

  • default → ping_interval=20.0, ping_timeout=20.0 (behavior unchanged)
  • ping_interval_s=None / ping_timeout_s=None → forwarded as None (keepalive disabled)

PYTHONPATH=src:envs pytest tests/test_core/test_generic_client.py -m "not integration" → 68 passed. Lint clean (ruff format --check, ruff check).

Claude Code Review

N/A


Note

Low Risk
Small, backward-compatible addition to core WebSocket connect options with targeted unit tests; no auth or data-path changes.

Overview
EnvClient (and subclasses like GenericEnvClient) now accept ping_interval_s and ping_timeout_s, which are passed through to ws_connect on connect. Defaults stay 20s for both, so behavior is unchanged unless callers override.

Setting either to None turns off client-side keepalive / pong timeout—needed when sandbox tunnels (e.g. Modal) drop WebSocket ping/pong frames and would otherwise drop the session on timeout.

Unit tests patch ws_connect to assert default forwarding and None when keepalive is disabled.

Reviewed by Cursor Bugbot for commit 3866684. Bugbot is set up for automated code reviews on this repo. Configure here.

@bot-ci-comment

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@burtenshaw

Copy link
Copy Markdown
Collaborator

Addressed in #878

@burtenshaw burtenshaw closed this Jun 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

core: make EnvClient WebSocket keepalive (ping_interval) configurable

2 participants