Fix/global proxy dispatcher#43919
Conversation
openclaw-barnacle
Bot
added
channel: whatsapp-web
Greptile SummaryThis PR fixes LLM inference timeouts for users behind proxies (corporate firewalls, restricted networks) by installing an undici
Confidence Score: 4/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/infra/net/proxy-env.ts
Line: 69
Comment:
**`socks4h://` not covered by SOCKS protocol regex**
The regex handles `socks4a://` and `socks5h://` but misses `socks4h://`, which is a valid SOCKS4a variant supported by curl and some proxy clients (Shadowrocket, etc.) for remote DNS resolution. Given the PR's stated goal of covering all major SOCKS variants, this is an easy edge case to add.
```suggestion
const SOCKS_PROTOCOL_RE = /^socks(?:4[ah]?|5h?)?:\/\//i;
```
The updated non-capturing group reads: `4[ah]?` matches `socks4`, `socks4a`, `socks4h`; `5h?` matches `socks5`, `socks5h`.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/infra/net/global-proxy.ts
Line: 42-47
Comment:
**"One-shot latch" comment contradicts actual retry behavior**
The JSDoc on `applied` says *"One-shot latch"* and *"proxy env changes require a full gateway restart"*, and the exported function docstring says *"only the first invocation takes effect"*. These statements are contradicted by the implementation (and explicitly tested): `applied` is **not** set when the early-return guard fires (no proxy vars) or when the constructor throws, so a second call can succeed in both scenarios.
The comment should reflect the real semantics — latch locks only on a successful application (or when a pre-existing proxy dispatcher is detected):
```suggestion
/**
* Success latch: set to `true` only after `EnvHttpProxyAgent` is
* successfully installed (or a compatible dispatcher is already present).
* A no-op early return (no proxy vars) or a constructor error leaves this
* `false`, allowing a subsequent call to retry.
*
* Proxy env changes after a successful application require a full gateway
* restart — the latch prevents redundant re-installation.
*/
let applied = false;
```
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: "fix(net): add global..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dafff91035
ℹ️ 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".
RickyTong1
force-pushed
the
fix/global-proxy-dispatcher
branch
from
dafff91 to
b4c5ff1
Compare
openclaw-barnacle
Bot
removed
the
channel: whatsapp-web
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7626662e2
ℹ️ 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".
RickyTong1
force-pushed
the
fix/global-proxy-dispatcher
branch
from
f762666 to
22b5399
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22b5399fa3
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2abda8782
ℹ️ 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".
RickyTong1
force-pushed
the
fix/global-proxy-dispatcher
branch
from
e2abda8 to
f5749de
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5749de77b
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ff1d32adb
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 049facf2f0
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5897b5a4d
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 452c1f0bf7
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24e9a83019
ℹ️ 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".
|
This problem has been bothering me for days. Good job~ |
RickyTong1
force-pushed
the
fix/global-proxy-dispatcher
branch
2 times, most recently
from
5394ef8 to
4623c35
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4623c35b46
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c677b2ba5d
ℹ️ 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".
openclaw-barnacle
Bot
added
the
channel: whatsapp-web
RickyTong1
force-pushed
the
fix/global-proxy-dispatcher
branch
from
bffb6d7 to
67775a7
Compare
openclaw-barnacle
Bot
added
size: M
and removed
channel: discord
RickyTong1
force-pushed
the
fix/global-proxy-dispatcher
branch
from
67775a7 to
263cc2d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 263cc2d70a
ℹ️ 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".
…d loopback bypass - Bootstrap undici EnvHttpProxyAgent at gateway startup when proxy env vars are detected (HTTP_PROXY, HTTPS_PROXY, ALL_PROXY) - Normalize socks4/4a/4h/5/5h:// URLs to http:// for undici compat - Route ALL_PROXY fallback through both httpProxy and httpsProxy - Ensure loopback addresses (localhost, 127.0.0.1, [::1]) are always bypassed via NO_PROXY merging; preserve NO_PROXY=* wildcard - Success-latch prevents redundant re-installation while allowing retry after no-op or constructor failure Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
RickyTong1
force-pushed
the
fix/global-proxy-dispatcher
branch
from
263cc2d to
a3b4b85
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3b4b856e0
ℹ️ 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".
| if (applied) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
In-process restarts re-enter startGatewayServer() (src/cli/gateway-cli/run-loop.ts:229-233), and each config read can add previously absent proxy vars from .env or config.env (src/config/io.ts:886, src/config/env-vars.ts:79-96). With this latch, those later values are never applied once the first proxy bootstrap succeeds. EnvHttpProxyAgent snapshots HTTP(S)_PROXY/NO_PROXY at construction time, so a gateway that first starts with only HTTP_PROXY will keep using that old proxy even after a restart adds HTTPS_PROXY or NO_PROXY, and users have to do a full process respawn for the change to take effect.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae7a965c96
ℹ️ 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".
| const rawAllProxy = process.env.all_proxy?.trim() || process.env.ALL_PROXY?.trim() || ""; | ||
| if (rawAllProxy && fallbackOptions.httpsProxy && fallbackOptions.httpsProxy !== rawAllProxy) { | ||
| log.warn( | ||
| `ALL_PROXY "${rawAllProxy}" rewritten to "${fallbackOptions.httpsProxy}" for undici compatibility — if your proxy only supports SOCKS, LLM requests will fail`, |
There was a problem hiding this comment.
If a user configures an authenticated SOCKS proxy via ALL_PROXY (for example socks5://user:pass@proxy:1080), this warning logs both the original and rewritten proxy URLs verbatim. Startup logs are often shared for debugging, so this change can leak proxy usernames/passwords into log files or issue reports. Please redact URL userinfo here before logging, similar to the existing URL-sanitizing helpers in the repo.
Useful? React with 👍 / 👎.
|
This pull request has been automatically marked as stale due to inactivity. |
|
Codex automated review: keeping this open. Keep this PR open. Current main has shipped the narrower HTTP_PROXY/HTTPS_PROXY EnvHttpProxyAgent bootstrap path, but it still intentionally excludes ALL_PROXY/all_proxy and SOCKS fallback behavior. That remaining scope is the central behavior this PR attempts to add, so it is not obsolete; it needs maintainer review, rebase, and security cleanup rather than conservative closure. Best possible solution: Keep this PR open for maintainer review, but require a rebase onto current main and a narrower implementation through the existing proxy-env and undici-global-dispatcher helpers. Add explicit tests for ALL_PROXY/all_proxy, socks4/socks4a/socks4h/socks5/socks5h normalization, lowercase-empty precedence, malformed and authenticated proxy URL redaction, NO_PROXY wildcard and loopback behavior, dispatcher replacement, and SSRF-preserving strict paths. If maintainers decide OpenClaw should intentionally keep Undici's HTTP_PROXY/HTTPS_PROXY-only semantics, close this only with that explicit product/security rationale and leave #43821 or a narrower follow-up as the canonical record. What I checked:
Remaining risk / open question:
Codex Review notes: model gpt-5.5, reasoning high; reviewed against fee16865b229. |
|
Thanks @RickyTong1. I carried this forward on main with a narrower core fix: What changed: Blacksmith Testbox verification:
I also attempted the broad Closing this PR because the behavior is now landed on main. |
3 participants
Summary
HTTP_PROXY/HTTPS_PROXY/ALL_PROXY), because Node.js 22+'sglobalThis.fetch(undici) does not read proxy env vars by default, and the upstreamstreamSimplein@mariozechner/pi-aidoes not accept adispatcherparameter.curl -x) proves the proxy itself is fine.applyGlobalProxyDispatcher()insrc/infra/net/global-proxy.tsthat callssetGlobalDispatcher(new EnvHttpProxyAgent())early in gateway startup (after dotenv loading), making allglobalThis.fetchcalls proxy-aware. AddedALL_PROXY/socks5:///socks5h://normalization insrc/infra/net/proxy-env.ts. 17 unit tests.Change Type
Scope
Linked Issue/PR
User-visible / Behavior Changes
HTTP_PROXY,HTTPS_PROXY, orALL_PROXYenv vars are set, LLM inference requests now route through the proxy instead of timing out.[net/global-proxy] global undici dispatcher set to EnvHttpProxyAgent (via ...)log line appears at startup when proxy is active.ALL_PROXYwithsocks5://orsocks5h://is rewritten tohttp://for undici compatibility (most local proxy tools like Clash/V2Ray accept HTTP CONNECT on the same port).NO_PROXY/no_proxyexclusions are respected — localhost services (Ollama, browser control, canvas host) are not affected.Security Impact (required)
NoNoYes— existingglobalThis.fetchcalls are now routed through the user-configured proxy when env vars are set. No new outbound calls are introduced.NoNoYes, explain risk + mitigation: The proxy dispatcher only activates when the user explicitly sets proxy env vars.EnvHttpProxyAgentrespectsNO_PROXYexclusions, so internal/localhost traffic is unaffected. IfEnvHttpProxyAgentconstruction fails (e.g. malformed URL), the error is logged and the gateway continues with direct connections.Repro + Verification
Environment
HTTP_PROXY=http://127.0.0.1:7897 HTTPS_PROXY=http://127.0.0.1:7897 ALL_PROXY=socks5://127.0.0.1:7897Steps
env HTTP_PROXY="http://127.0.0.1:7897" HTTPS_PROXY="http://127.0.0.1:7897" openclaw gatewayExpected
[net/global-proxy] global undici dispatcher set to EnvHttpProxyAgent (via HTTP_PROXY)in startup logActual (before fix)
LLM request timed outerrors, even thoughcurl -x http://127.0.0.1:7897 https://generativelanguage.googleapis.com/...succeedsEvidence
Unit tests (17 cases, all passing):
Regression tests pass with zero regressions:
src/infra/net/proxy-fetch.test.ts— 7/7 passedsrc/infra/net/fetch-guard.ssrf.test.ts— 9/9 passedStartup log confirming proxy activation:
Human Verification
npm link(OpenClaw 2026.3.13, commit 2b46c4c)env OPENCLAW_DISABLE_BONJOUR=1 HTTP_PROXY="http://127.0.0.1:7897" HTTPS_PROXY="http://127.0.0.1:7897" ALL_PROXY="socks5://127.0.0.1:7897" openclaw gateway run --force[net/global-proxy] global undici dispatcher set to EnvHttpProxyAgent (via HTTP_PROXY)in startup logsocks5h://protocol variant (was broken by original regex, now fixed)ALL_PROXY-only configuration (noHTTP_PROXY/HTTPS_PROXYset)Review Conversations
Compatibility / Migration
YesNo— uses existing standard proxy env vars (HTTP_PROXY,HTTPS_PROXY,ALL_PROXY,NO_PROXY)NoFailure Recovery (if this breaks)
applyGlobalProxyDispatcher()call insrc/gateway/server.impl.ts(single line). Or simply unset proxy env vars — the function is a no-op without them.src/gateway/server.impl.ts(remove import + call)socks5://tohttp://will cause connection failures. Users would see fetch errors instead of timeouts.Risks and Mitigations
globalThis.fetchcalls process-wide.streamSimplewhich only usesglobalThis.fetch.EnvHttpProxyAgentrespectsNO_PROXYso internal traffic is excluded. The function is cooperative (skips if a proxy dispatcher is already installed) and idempotent.Comparison with existing open PRs
ProxyAgentinserver-startup.tsNO_PROXY; modifies Telegram codeEnvHttpProxyAgentinundici-global-dispatcher.tsALL_PROXY; only covers embedded runnerFiles changed
src/infra/net/global-proxy.ts— New module:applyGlobalProxyDispatcher()+ test reset helpersrc/infra/net/global-proxy.test.ts— 17 unit testssrc/infra/net/proxy-env.ts— AddedresolveAllProxyFallbackOptions(), SOCKS protocol normalization (builds onresolveEnvHttpProxyUrl/hasEnvHttpProxyConfiguredfrom Fix env proxy bootstrap for model traffic #43248)src/gateway/server.impl.ts— Import and callapplyGlobalProxyDispatcher()afterreadConfigFileSnapshot()Relationship with #43248
#43248 (merged 2026-03-11) added
ensureGlobalUndiciEnvProxyDispatcher()inundici-global-dispatcher.ts, called inside the embedded runner'srunEmbeddedAttempt(). This PR complements it with two things #43248 does not cover:runEmbeddedAttempt()— per-run, embedded runner onlystartGatewayServer()— once at gateway startup, covers all fetch pathsglobalThis.fetch: LLM inference, heartbeat, web search, usage tracking, etc.ALL_PROXYsupportresolveAllProxyFallbackOptions()bridges undici'sEnvHttpProxyAgentgapsocks5://,socks5h://,socks4://rewritten tohttp://readConfigFileSnapshot()so.env-defined proxy vars are visibleThis PR reuses the
resolveEnvHttpProxyUrlandhasEnvHttpProxyConfiguredhelpers that #43248 added toproxy-env.ts, and addsresolveAllProxyFallbackOptions()on top for the ALL_PROXY/SOCKS gap.