Skip to content

fix(net): honor HTTP(S)_PROXY in embedded runner dispatcher#41455

Closed
nowanti wants to merge 1 commit intoopenclaw:mainfrom
nowanti:fix/embedded-runner-proxy-env-proxy-agent
Closed

fix(net): honor HTTP(S)_PROXY in embedded runner dispatcher#41455
nowanti wants to merge 1 commit intoopenclaw:mainfrom
nowanti:fix/embedded-runner-proxy-env-proxy-agent

Conversation

@nowanti
Copy link
Copy Markdown

@nowanti nowanti commented Mar 9, 2026

Summary

  • Problem: ensureGlobalUndiciStreamTimeouts() kept a plain undici Agent even when HTTP_PROXY / HTTPS_PROXY were configured.
  • Why it matters: embedded runner requests such as openai-codex/gpt-5.4 could still try direct connections and time out behind HTTP proxies.
  • What changed: when HTTP_PROXY / HTTPS_PROXY are present, the dispatcher now upgrades Agent -> EnvHttpProxyAgent, and it re-applies the extended timeouts if another subsystem replaces that global dispatcher. Tests cover the upgrade path, the re-apply path, and the ALL_PROXY-only non-upgrade path.
  • What did NOT change (scope boundary): this PR does not add ALL_PROXY support and does not change Telegram-specific proxy selection logic.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

User-visible / Behavior Changes

  • Before: embedded runner requests could still time out with LLM request timed out even when HTTP_PROXY / HTTPS_PROXY were set.
  • After: embedded runner requests use EnvHttpProxyAgent when HTTP_PROXY / HTTPS_PROXY are set.
  • After: if another subsystem replaces the global env-proxy dispatcher, embedded runner setup restores the extended stream timeouts on the next run.
  • Out of scope: ALL_PROXY-only setups are unchanged.

Security Impact (required)

  • New permissions/capabilities? (No)
  • Secrets/tokens handling changed? (No)
  • New/changed network calls? (No)
  • Command/tool execution surface changed? (No)
  • Data access scope changed? (No)
  • If any Yes, explain risk + mitigation:

Repro + Verification

Environment

  • OS: macOS 14.x
  • Runtime/container: Node.js v24.14.0
  • Model/provider: openai-codex/gpt-5.4
  • Integration/channel (if any): Gateway-backed agent CLI
  • Relevant config (redacted):
    {
      "agents": {
        "defaults": {
          "model": {
            "primary": "openai-codex/gpt-5.4"
          }
        }
      }
    }

Steps

  1. Set agents.defaults.model.primary to openai-codex/gpt-5.4.
  2. Run OpenClaw with working HTTP_PROXY / HTTPS_PROXY values.
  3. Run:
    openclaw agent --session-id proxy-check-gpt54-gateway --message "Reply with only OK." --json --timeout 120

Expected

  • Embedded runner traffic uses the configured HTTP proxy.
  • The openai-codex/gpt-5.4 turn completes successfully.

Actual

  • Before fix: embedded run agent end: ... error=LLM request timed out.
  • After fix: the gateway-backed agent turn completes with status: ok and reply OK.

Evidence

Attach at least one:

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Log Before Fix

warn agent/embedded {"subsystem":"agent/embedded"} embedded run agent end: runId=... isError=true error=LLM request timed out.

Log / Command After Fix

$ openclaw agent --session-id proxy-check-gpt54-gateway --message "Reply with only OK." --json --timeout 120
{
  "status": "ok",
  "result": {
    "payloads": [
      {
        "text": "OK"
      }
    ]
  }
}

Unit Tests

$ pnpm exec vitest run src/infra/net/undici-global-dispatcher.test.ts
✓ src/infra/net/undici-global-dispatcher.test.ts (8 tests)

Human Verification (required)

What you personally verified (not just CI), and how:

  • Verified scenarios:
    • HTTP_PROXY / HTTPS_PROXY configured on macOS
    • gateway-backed openai-codex/gpt-5.4 turn succeeds
    • unit tests for dispatcher upgrade behavior pass
  • Edge cases checked:
    • existing EnvHttpProxyAgent remains on env-proxy path
    • an externally replaced EnvHttpProxyAgent is re-applied with extended timeouts
    • ALL_PROXY-only setup does not upgrade in this PR
    • custom ProxyAgent remains untouched
  • What you did not verify:
    • ALL_PROXY support
    • Windows
    • Docker / containerized installs
    • non-OpenAI embedded providers

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.

Compatibility / Migration

  • Backward compatible? (Yes)
  • Config/env changes? (No)
  • Migration needed? (No)
  • If yes, exact upgrade steps:

Failure Recovery (if this breaks)

  • How to disable/revert this change quickly: revert this commit
  • Files/config to restore:
    • src/infra/net/proxy-env.ts
    • src/infra/net/undici-global-dispatcher.ts
    • src/infra/net/undici-global-dispatcher.test.ts
  • Known bad symptoms reviewers should watch for:
    • HTTP_PROXY / HTTPS_PROXY configured but embedded runner traffic still behaves like direct-connect
    • unexpected dispatcher changes when a custom proxy dispatcher is already installed

Risks and Mitigations

  • Risk: readers may assume this also fixes ALL_PROXY-only environments.
    • Mitigation: scope is explicitly limited in code, tests, and this PR description to HTTP_PROXY / HTTPS_PROXY.
  • Risk: changing the default dispatcher path could affect embedded runner networking behavior.
    • Mitigation: the change only applies when a plain Agent is active and HTTP_PROXY / HTTPS_PROXY are already configured; custom proxy dispatchers remain untouched, and tests cover re-application after an external dispatcher swap.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 9, 2026

Greptile Summary

This PR fixes a bug where ensureGlobalUndiciStreamTimeouts() always created a plain undici Agent as the global dispatcher, even when HTTP_PROXY / HTTPS_PROXY were set in the environment — causing embedded runner requests (e.g., openai-codex/gpt-5.4) to bypass the configured proxy and time out.

Changes:

  • proxy-env.ts: Adds HTTP_PROXY_ENV_KEYS constant (excluding ALL_PROXY) and a hasHttpProxyEnvConfigured() helper that returns true only when HTTP_PROXY, HTTPS_PROXY, http_proxy, or https_proxy is non-empty.
  • undici-global-dispatcher.ts: When the current dispatcher is a plain Agent and hasHttpProxyEnvConfigured() returns true, the resolved nextKind is upgraded from "agent" to "env-proxy", causing the function to install an EnvHttpProxyAgent with stream timeouts instead.
  • undici-global-dispatcher.test.ts: Adds vi.unstubAllEnvs() + blank stubs to beforeEach for test isolation, and adds two new test cases covering the upgrade path (HTTP/HTTPS proxy set) and the non-upgrade path (ALL_PROXY-only).

The fix is minimal, well-scoped, and backward-compatible. Existing dispatchers of types other than plain Agent remain untouched. The idempotency cache correctly incorporates nextKind, so proxy-env changes between calls are detected and acted on.

Confidence Score: 5/5

  • This PR is safe to merge — it is a minimal, targeted bug fix with no breaking changes.
  • The change is small (one conditional expression + one helper function + test coverage). The upgrade path only fires when the current dispatcher is a plain Agent AND HTTP_PROXY/HTTPS_PROXY are non-empty, leaving every other dispatcher type untouched. The idempotency cache key correctly incorporates nextKind, so env changes between calls are handled properly. Tests cover both the upgrade path and the ALL_PROXY-only non-upgrade path, and beforeEach isolation prevents ambient env vars from polluting results.
  • No files require special attention.

Last reviewed commit: c8f1516

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c8f1516f88

ℹ️ 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".

Comment thread src/infra/net/undici-global-dispatcher.ts Outdated
…s configured

Ensure that ensureGlobalUndiciStreamTimeouts() detects when HTTP_PROXY/
HTTPS_PROXY env vars are configured and upgrades the undici global dispatcher
from plain Agent to EnvHttpProxyAgent. This fixes the issue where gpt-5.4
(openai-codex) and other embedded runners timeout behind proxies because
the default Agent ignores proxy settings.

Changes:
- Check hasProxyEnvConfigured() when determining next dispatcher kind
- Upgrade 'agent' -> 'env-proxy' when proxy env vars are present
- Update tests to verify the upgrade behavior

Fixes openclaw#41046 (related)
@nowanti nowanti force-pushed the fix/embedded-runner-proxy-env-proxy-agent branch from c8f1516 to 095f515 Compare March 9, 2026 21:53
@RickyTong1 RickyTong1 mentioned this pull request Mar 12, 2026
20 tasks
@nowanti
Copy link
Copy Markdown
Author

nowanti commented Mar 13, 2026

Rebased this branch onto the current main and verified it now matches upstream exactly, so this PR no longer contains any diff.

This fix path has already landed upstream in #43248 (87876a3e3), which covers the env-proxy bootstrap / timeout flow on the latest code.

Closing this PR as superseded to avoid duplicate review churn.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant