fix(image-gen): bypass macOS system proxy for custom endpoints - #64893
fix(image-gen): bypass macOS system proxy for custom endpoints#64893asdlem wants to merge 2 commits into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for carrying the proxy policy into the OpenAI image provider. The production change targets a live gap: current main still constructs this client with openai.OpenAI() at plugins/image_gen/openai/__init__.py:273, while agent/process_bootstrap.py:151-157 documents the shared transport's macOS system-proxy safeguard.
Problems
tests/plugins/image_gen/test_openai_provider.py:122patches macOS proxy discovery but leaves explicitHTTP(S)_PROXY/ALL_PROXYvariables intact. The helper intentionally honors those variables (agent/process_bootstrap.py:153-196), so the assertion can fail in a proxy-configured test runner. The analogous regression test explicitly clears them attests/agent/test_auxiliary_client_proxy_env.py:27-29.
Suggested changes
- Clear upper- and lowercase explicit proxy variables, plus
NO_PROXYvariants, before the patchedgetproxies()call. That makes the test specifically verify system-proxy bypass.
Automated hermes-sweeper review.
| @@ -120,6 +121,24 @@ def test_config_top_level_model(self, tmp_path): | |||
| assert model_id == "gpt-image-2-high" | |||
| assert meta["quality"] == "high" | |||
There was a problem hiding this comment.
Please clear explicit HTTPS_PROXY/HTTP_PROXY/ALL_PROXY (including lowercase and NO_PROXY variants) before this test. build_keepalive_http_client() intentionally honors explicit proxy env vars, so this assertion can fail on a proxy-configured runner even though the macOS system-proxy bypass works. See tests/agent/test_auxiliary_client_proxy_env.py:27-29 for the established isolation pattern.
The macOS system proxy bypass test previously only mocked urllib.request.getproxies() but left explicit HTTP_PROXY / HTTPS_PROXY / ALL_PROXY env vars intact. The shared transport helper intentionally honors those variables (agent/process_bootstrap.py:153-196), so a proxy-configured test runner could cause the assertion to fail. Mirror the pattern from tests/agent/test_auxiliary_client_proxy_env.py and clear upper- and lowercase proxy env vars before the assertion, so the test specifically verifies system-proxy bypass. Review: NousResearch#64893 (review)
SummaryOne PR addresses #64888. #64893 changes the image provider to use Related pull requests
Suggested consolidationKeep #64893 open with a salvage path: retain the targeted Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I64888(["issue #64888 (open)"])
P64893["PR #64893 (open)"]
P64893 -->|best fix| I64888
class I64888 open
class P64893 open
class P64893 best
class P64893 target
click I64888 "https://github.com/NousResearch/hermes-agent/issues/64888"
click P64893 "https://github.com/NousResearch/hermes-agent/pull/64893"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 3 kB of PR diffs, 7 kB of issue/PR text, 1 kB of discussion (1 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
What does this PR do?
The OpenAI image generation provider currently creates its SDK client with the default environment-aware httpx transport. On macOS, that can route a custom
OPENAI_BASE_URLthrough the system proxy even when the endpoint is listed in the system proxy exceptions, because those exceptions are not exposed throughurllib.request.getproxies().This change reuses Hermes' existing
build_keepalive_http_client()policy whenever a custom image endpoint is configured. The default OpenAI client behavior remains unchanged whenOPENAI_BASE_URLis not set.Related Issue
Fixes #64888
Type of Change
Changes Made
plugins/image_gen/openai/__init__.pyto inject Hermes' custom httpx client whenOPENAI_BASE_URLis configured.HTTPProxytransport.How to Test
OPENAI_BASE_URLto a local or custom OpenAI-compatible endpoint.scripts/run_tests.sh tests/plugins/image_gen/test_openai_provider.py -qand verify all 30 tests pass.python3 scripts/check-windows-footguns.py plugins/image_gen/openai/__init__.py tests/plugins/image_gen/test_openai_provider.py.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/A: no user-facing configuration or behavior contract changedcli-config.yaml.exampleif I added/changed config keys — or N/A: no config keys changedCONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/A: no architecture or workflow changesScreenshots / Logs
Targeted verification:
scripts/run_tests.sh tests/plugins/image_gen/test_openai_provider.py -q: 30 passedgit diff --check: passed