Skip to content

feat: configurable retryPolicy and honour all_proxy in env proxy - #7

Open
AlienHub wants to merge 1 commit into
suntianc:mainfrom
AlienHub:fix/retry-policy-all-proxy
Open

feat: configurable retryPolicy and honour all_proxy in env proxy#7
AlienHub wants to merge 1 commit into
suntianc:mainfrom
AlienHub:fix/retry-policy-all-proxy

Conversation

@AlienHub

Copy link
Copy Markdown

Problem

Two pain points when using the openai-codex route on real networks:

  1. Retry count is not configurable. CodexAuthAdapter hardcodes
    resolveRetryPolicy(undefined, …), so the route always uses the harness
    normal policy (max 2 retries). On a flaky/rate-limited
    chatgpt.com/backend-api, requests frequently need more attempts and there
    is no way to raise the budget without editing the installed package.

  2. all_proxy is ignored. installEnvHttpProxy only reads
    HTTPS_PROXY/https_proxy/HTTP_PROXY/http_proxy. Many proxy setups
    (e.g. Clash) set only all_proxy, which curl and the codex CLI honour —
    but Node's undici does not, and new EnvHttpProxyAgent() also reads only
    http_proxy/HTTPS_PROXY. On such networks every request to
    chatgpt.com went direct and timed out (connect timeout), while the
    browser/CLI worked fine.

Changes

  • Configurable retry policy: optional retryPolicy on the
    llm-codex-auth row (validated with RetryPolicySchema), threaded through
    CodexAuthAdapterOptions into resolveRetryPolicy. Omitted ⇒ unchanged
    default (normal, 2 retries). Example:

    - id: llm-codex-auth
      config:
        retryPolicy:
          mode: normal
          maxRetries: 5
  • all_proxy support: the env-proxy chain now includes
    ALL_PROXY/all_proxy, and the resolved URL is passed explicitly as
    { httpProxy, httpsProxy } (undici's env parser never reads all_proxy).
    NO_PROXY behaviour is preserved from the environment.

  • README (en/zh) config table and CHANGELOG updated.

Validation

  • pnpm run check passes: oxlint, typecheck (both host and client, with
    exactOptionalPropertyTypes), vitest, build, package smoke, publint.
  • On a machine where all_proxy=http://127.0.0.1:7890 is the only proxy env
    var: undici direct to chatgpt.com/backend-api times out; with the patched
    dispatcher the same request succeeds through the proxy (HTTP 401, as
    expected without auth headers).

- Add optional retryPolicy to the llm-codex-auth row (RetryPolicySchema);
  threaded through CodexAuthAdapterOptions to resolveRetryPolicy. Default
  behavior unchanged (harness normal policy, two retries).
- installEnvHttpProxy now also reads ALL_PROXY/all_proxy and passes the
  resolved URL explicitly to EnvHttpProxyAgent, since undici's env parser
  ignores all_proxy. Fixes connect timeouts on networks that only set
  all_proxy (e.g. Clash).
- Document the new retryPolicy field in README (en/zh) and CHANGELOG.
@suntianc

Copy link
Copy Markdown
Owner

Thanks for the contribution — both directions are useful. Configurable retry behavior is valuable on flaky/rate-limited networks, and supporting ALL_PROXY/all_proxy addresses a real gap between Codex CLI/curl and Undici.

A few changes are needed before this can merge:

  1. Please rebase onto the current main. The PR currently conflicts in CHANGELOG.md and src/codex-auth-adapter.ts. When resolving the adapter conflict, please preserve the newer auth: codexAuthInjection() and maxRequestImageBytes behavior from main.
  2. Preserve protocol-specific proxy semantics. The current code resolves one URL and passes it as both httpProxy and httpsProxy. This collapses distinct http_proxy/https_proxy settings, can route HTTP through the HTTPS proxy, and changes Undici's lowercase-before-uppercase precedence. Please resolve HTTP and HTTPS independently, using all_proxy/ALL_PROXY only as the fallback for each, while continuing to leave NO_PROXY to EnvHttpProxyAgent.
  3. Update the documented default. Current main uses @deepseek-ai/dsh-llm@0.1.1-rc.1, where resolveRetryPolicy(undefined) defaults to normal mode with maxRetries: 5, not 2. Delegating the omitted value is still correct; the README, changelog, and source comments should say 5 or avoid hard-coding the number.
  4. Please add tests for retry-policy schema/default/forwarding and for proxy precedence, all_proxy fallback, and NO_PROXY preservation. The isolated PR-head pnpm run check passes, but the new behavior is currently untested.

So: the feature direction is worth keeping; the main blockers are the stale base and proxy-resolution regression.

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.

2 participants