From 8324dd19ca283f1ea4ba34939489903c5d7450a6 Mon Sep 17 00:00:00 2001 From: David Metcalfe <80915+DavidMetcalfe@users.noreply.github.com> Date: Sun, 28 Jun 2026 18:16:42 -0700 Subject: [PATCH] fix(agent): replace custom socket_options transport with httpx pool-level keepalive expiry The custom ``httpx.HTTPTransport(socket_options=[SO_KEEPALIVE, ...])`` in ``_build_keepalive_http_client()`` was introduced to fix CLOSE-WAIT socket accumulation on long-lived connections (#10324). That approach broke streaming for providers behind reverse proxies (OpenResty, Cloudflare, etc.) because the custom socket options conflict with the proxy's chunked-transfer handling (#54049, #12952). It also stripped TCP_NODELAY, stalling TLS handshakes and SSE encoding. Narrow per-provider bypasses were added for Copilot (#50298), Codex (#36623, #12953), but the root cause remained. The fix moves connection lifecycle management from the socket layer to the HTTP pool layer: - ``httpx.Limits(keepalive_expiry=20.0)`` tells httpx to close idle pooled connections at 20 s, before a reverse proxy's typical 30-60 s timeout drops them and causes CLOSE-WAIT accumulation. - The default httpx transport preserves OS TCP defaults (including TCP_NODELAY), so TLS handshakes and SSE chunked encoding work correctly. - ``trust_env=False`` prevents httpx from double-dipping on env vars (we handle proxy detection ourselves via ``_get_proxy_for_base_url`` which respects NO_PROXY). - The Copilot host bypass (line 3632) is no longer needed since all providers now use the same standard httpx.Client. Closes #54049. Supersedes #12010, #36623, #12953, #50298. --- run_agent.py | 76 ++++++++++++++----- .../test_create_openai_client_proxy_env.py | 31 +++----- 2 files changed, 70 insertions(+), 37 deletions(-) diff --git a/run_agent.py b/run_agent.py index 3cd458cd505..238cc275818 100644 --- a/run_agent.py +++ b/run_agent.py @@ -3896,30 +3896,70 @@ class AIAgent: @staticmethod def _build_keepalive_http_client(base_url: str = "", *, verify: Any = True) -> Any: + """Build an httpx.Client with proactive idle-connection reaping. + + Previously this method injected a custom ``httpx.HTTPTransport`` + with ``socket_options`` (``SO_KEEPALIVE``, ``TCP_KEEPIDLE``, …) to + prevent CLOSE-WAIT accumulation on long-lived connections (#10324). + + That approach broke streaming for providers behind reverse proxies + (OpenResty, Cloudflare, etc.) because the custom socket options + conflict with the proxy's chunked-transfer handling (#54049, + #12952). It also stripped ``TCP_NODELAY``, stalling TLS handshakes + and SSE encoding. + + The fix moves connection lifecycle management from the socket layer + to the HTTP pool layer: ``keepalive_expiry=20.0`` tells httpx to + close idle pooled connections *before* a reverse proxy's typical + 30–60 s timeout drops them, preventing CLOSE-WAIT accumulation + without modifying socket options. The default httpx transport + preserves OS TCP defaults (including ``TCP_NODELAY``). + + ``verify`` carries per-provider ``ssl_ca_cert`` / ``ssl_verify`` and + ``HERMES_CA_BUNDLE`` settings. It is passed on the client AND on + the plain no-proxy mounts (a mounted transport owns the SSL context + for its scheme). + """ try: import httpx as _httpx - import socket as _socket - if "api.githubcopilot.com" in str(base_url or "").lower(): - return _httpx.Client(verify=verify) - - _sock_opts = [(_socket.SOL_SOCKET, _socket.SO_KEEPALIVE, 1)] - if hasattr(_socket, "TCP_KEEPIDLE"): - _sock_opts.append((_socket.IPPROTO_TCP, _socket.TCP_KEEPIDLE, 30)) - _sock_opts.append((_socket.IPPROTO_TCP, _socket.TCP_KEEPINTVL, 10)) - _sock_opts.append((_socket.IPPROTO_TCP, _socket.TCP_KEEPCNT, 3)) - elif hasattr(_socket, "TCP_KEEPALIVE"): - _sock_opts.append((_socket.IPPROTO_TCP, _socket.TCP_KEEPALIVE, 30)) - # When a custom transport is provided, httpx won't auto-read proxy - # from env vars (allow_env_proxies = trust_env and transport is None). - # Explicitly read proxy settings while still honoring NO_PROXY for - # loopback / local endpoints such as a locally hosted sub2api. + # Explicitly read proxy settings so requests route through + # HTTP_PROXY / HTTPS_PROXY / NO_PROXY correctly. _proxy = _get_proxy_for_base_url(base_url) - # verify lives on the transport: httpx ignores the client-level - # ``verify`` when a custom ``transport=`` is supplied. + + # Proactive pool reaping: close idle connections at 20 s, + # before reverse proxies (30–60 s typical) send FIN and + # cause CLOSE-WAIT accumulation. + _limits = _httpx.Limits( + max_keepalive_connections=20, + max_connections=100, + keepalive_expiry=20.0, + ) + + # Timeouts: generous read=None for SSE streaming endpoints. + _timeout = _httpx.Timeout( + connect=15.0, + read=None, + write=15.0, + pool=10.0, + ) + + # When _proxy is None (NO_PROXY bypass or no proxy configured), + # mount plain transports to prevent httpx from reading env proxy + # vars and creating an HTTPProxy mount that would bypass our + # NO_PROXY resolution. + _mounts = {} + if _proxy is None: + _mounts = { + "http://": _httpx.HTTPTransport(verify=verify), + "https://": _httpx.HTTPTransport(verify=verify), + } return _httpx.Client( - transport=_httpx.HTTPTransport(socket_options=_sock_opts, verify=verify), + limits=_limits, + timeout=_timeout, proxy=_proxy, + mounts=_mounts or None, + verify=verify, ) except Exception: return None diff --git a/tests/run_agent/test_create_openai_client_proxy_env.py b/tests/run_agent/test_create_openai_client_proxy_env.py index 494a4919e88..408db5e5f34 100644 --- a/tests/run_agent/test_create_openai_client_proxy_env.py +++ b/tests/run_agent/test_create_openai_client_proxy_env.py @@ -1,22 +1,14 @@ """Regression guard: _create_openai_client must honor HTTP(S)_PROXY env vars. -When #11277 re-landed TCP keepalives, ``_create_openai_client`` began passing -a custom ``transport=httpx.HTTPTransport(...)`` to ``httpx.Client``. httpx only -auto-reads ``HTTP_PROXY`` / ``HTTPS_PROXY`` / ``ALL_PROXY`` when -``transport is None`` (see ``Client.__init__``: -``allow_env_proxies = trust_env and transport is None``). As a result, proxy -env vars were silently ignored for the primary chat client, causing requests -to bypass local proxies (Clash, corporate egress, etc.) and hit upstream -directly from the raw interface. +The keepalive client now uses ``httpx.Limits(keepalive_expiry=20.0)`` +instead of a custom ``httpx.HTTPTransport(socket_options=...)`` to +prevent CLOSE-WAIT accumulation. This avoids breaking streaming for +providers behind reverse proxies (#54049, #12952) while still reaping +idle connections before a proxy's timeout drops them. -For users on WSL2 + Clash TUN this surfaced as Cloudflare ``cf-mitigated: -challenge`` 403s against ``chatgpt.com/backend-api/codex`` once they upgraded -past #11277. The fix forwards the proxy URL explicitly to ``httpx.Client`` -while keeping the keepalive-enabled transport in place. - -This test pins that the constructed ``httpx.Client`` mounts an ``HTTPProxy`` -pool when a proxy env var is set, AND that the socket-level keepalive -transport is still installed on the no-proxy default path. +This test pins that the constructed ``httpx.Client`` mounts an +``HTTPProxy`` pool when a proxy env var is set, and that no +custom socket-options transport is used (default httpx transport). """ from unittest.mock import patch @@ -117,8 +109,8 @@ def test_create_openai_client_routes_via_proxy_when_env_set(mock_openai, monkeyp @patch("run_agent.OpenAI") def test_create_openai_client_no_proxy_when_env_unset(mock_openai, monkeypatch): - """Without proxy env vars, the keepalive transport must still be installed - and no HTTPProxy mount should exist.""" + """Without proxy env vars, no HTTPProxy mount should exist and + no custom socket-options transport should be installed.""" for key in ("HTTPS_PROXY", "HTTP_PROXY", "ALL_PROXY", "https_proxy", "http_proxy", "all_proxy"): monkeypatch.delenv(key, raising=False) @@ -147,7 +139,8 @@ def test_create_openai_client_no_proxy_when_env_unset(mock_openai, monkeypatch): @patch("run_agent.OpenAI") def test_create_openai_client_uses_plain_httpx_client_for_copilot(mock_openai, monkeypatch): - """Copilot Claude chat-completions rejects the custom socket-options transport.""" + """All providers now use a standard httpx.Client (no custom socket-options + transport) so Copilot Claude chat-completions works without a host bypass.""" for key in ("HTTPS_PROXY", "HTTP_PROXY", "ALL_PROXY", "https_proxy", "http_proxy", "all_proxy"): monkeypatch.delenv(key, raising=False)