fix: broken HTTP proxy for providers with custom logic by kevincojean · Pull Request #132 · b3nw/LLM-API-Key-Proxy

kevincojean · 2026-09-03T10:02:12Z

HTTP / SOCKS proxies didn't work for providers that build their own HTTP client.

The previous fix only helped providers that go through _resolve_litellm_client.
Four providers that build their own openai.AsyncOpenAI(http_client=client) silently bypassed the proxy.
Streaming was even worse because _resolve_http_client was never called.

Providers for which has_custom_logic() is True are responsible for how they instanciate their HTTP clients and may chose (or forget) to respect network proxy support.

…igured proxy The previous proxy fix only helped providers that route through _resolve_litellm_client (has_custom_logic() == False). The 4 providers that build their own openai.AsyncOpenAI(http_client=client) - opencode_go, cline_pass, ollama_cloud, x_ai - silently bypassed every configured proxy because RequestExecutor._execute_non_streaming and _execute_streaming passed their unproxied shared client (self._http_client) to the plugin's acompletion(client=...). The plugin faithfully wired that unproxied client into its own AsyncOpenAI, so HTTPS traffic went straight to the upstream. _resolve_http_client was called and the proxy-aware httpx.AsyncClient was returned, but the executor then discarded it. The streaming path was worse: _resolve_http_client was never even called there, so every streaming request bypassed the proxy regardless of plugin. The 12-hex stable_id format users observed (e.g. AF9907BBF840) was correct all along - SHA-256 of the API key, truncated to 12 chars. The bug was purely in the executor, not in the lookup or env-var naming. Changes: - executor.py (_execute_non_streaming): pass the already-resolved request_client to plugin.acompletion / plugin.aembedding instead of self._http_client. - executor.py (_execute_streaming): resolve request_client_s via _resolve_http_client before the retry loop (it was missing entirely) and pass it to plugin.acompletion instead of self._http_client. - tests/test_proxy_request_routing.py: E2E test that spins up a real local TCP socket as a fake CONNECT proxy (no mocking at the network seam) and asserts the executor's outbound CONNECT lands on it when PROXY_URL_CREDENTIAL_<id> is configured. Covers non-streaming and streaming. Both go RED before the fix and GREEN after. - .fork/stack.yml: register this as the third proxy feature commit. - .fork/features/proxy.md: document the bug, fix, and scope note. Scope note: only the 4 providers that build their own AsyncOpenAI(http_client=client) now route through the proxy. The remaining has_custom_logic() == True providers do not consume the passed client at all (opencode_zen explicitly notes "# client unused" in source). For those, proxy support requires provider-specific refactors and remains out of scope here. Verified locally: - py_compile + ruff F401/F811/F821/E9 on all touched .py files - 502 passed in full suite, 2 pre-existing failures unrelated to this change (hardcoded 2026-07-01 timestamp now in the past; HTTP 400 vs 200 on unrelated test) confirmed by stashing patch + rerun

…igured proxy

Copilot

🟡 Changes recommended

The streaming path now double-resolves proxy selection (spec + client), which can advance rotation and/or misreport proxy_name when proxy rotation is enabled.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR fixes outbound proxy routing for providers that implement has_custom_logic() by ensuring the executor passes the proxy-aware httpx.AsyncClient into provider plugins for both non-streaming and streaming paths.

Changes:

Pass the resolved request_client (proxy-aware) into custom-logic plugins in _execute_non_streaming.
Resolve a proxy-aware client in _execute_streaming and pass it into custom-logic plugins.
Add an end-to-end regression test that spins up a local fake CONNECT proxy and asserts it is contacted for opencode_go and deepseek.

File summaries

File	Description
`src/rotator_library/client/executor.py`	Routes custom-logic plugin calls through the resolved proxy-aware HTTP client (non-streaming + streaming).
`tests/test_proxy_request_routing.py`	Adds E2E regression tests using a local fake CONNECT proxy to validate proxy routing reaches the network seam.
`.gitignore`	Un-ignores the new test file so it is tracked alongside other explicitly-included tests.
`.fork/stack.yml` / `.fork/features/proxy.md`	Records the fix in the fork’s stack/feature ledger.

Review details

Files reviewed: 4/5 changed files
Comments generated: 3
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

chore(client/executor): update _resolve_http_client to accept resolved_spec refactor(client/executor): reuse proxy_spec in _

socket-security · 2026-09-03T10:55:13Z

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	pypi/pytest@9.1.1
	pypi/pytest-asyncio@1.4.0

View full report

github-actions · 2026-09-05T14:30:15Z

 !tests/test_access_log_middleware.py
 !tests/test_gemini_cli_models.py
 !tests/test_suppress_litellm_warnings.py
+!tests/test_proxy_request_routing.py


Remove the inline comment because Git treats it as part of the pattern, preventing this test file from being unignored.

Suggested change

!tests/test_proxy_request_routing.py

!tests/test_proxy_request_routing.py

#ai-review-inline

github-actions · 2026-09-05T14:30:20Z


 [dependency-groups]
 dev = [
+    "pytest>=9.1.1",


Remove the temporary “added” annotations because they provide no lasting context and will quickly become stale.

Suggested change

"pytest>=9.1.1",

"pytest>=9.1.1",

"pytest-asyncio>=1.4.0",

#ai-review-inline

github-actions · 2026-09-05T14:30:30Z

    ) -> httpx.AsyncClient:
        """Get or create an httpx client for the resolved proxy."""
-        spec = self.config.resolve(provider, credential, stable_id)
+        spec = (


Remove the per-line change-tracking comments to keep the fallback logic readable.

Suggested change

spec = (

spec = (

resolved_spec

if resolved_spec is not None

else self.config.resolve(provider, credential, stable_id)

)

#ai-review-inline

github-actions · 2026-09-05T14:30:30Z

        provider: str,
        credential: str,
        stable_id: str,
+        resolved_spec: Optional[ProxySpec] = None,


Remove the change-tracking comment because version control already records this addition.

Suggested change

resolved_spec: Optional[ProxySpec] = None,

resolved_spec: Optional[ProxySpec] = None,

#ai-review-inline

github-actions · 2026-09-05T14:30:30Z

+                provider,
+                credential,
+                stable_id,
+                resolved_spec=resolved_spec,


Using None as both the default and a valid no-proxy result may cause the pool to resolve a proxy again; use a sentinel to distinguish an omitted spec.

#ai-review-inline

github-actions · 2026-09-05T14:30:30Z

+                provider,
+                credential,
+                stable_id,
+                resolved_spec=resolved_spec,


This new keyword argument will break client-pool implementations that still use the previous three-argument get_client signature; update the shared interface and all implementations.

#ai-review-inline

github-actions · 2026-09-05T14:30:54Z

+
+
+DEEPSEEK_TEST_API_KEY = "sk-fake-deepseek-key-for-proxy-routing-0000"
+DEEPSEEK_TEST_STABLE_ID = hashlib.sha256(DEEPSEEK_TEST_API_KEY.encode()).hexdigest()[:12]


DEEPSEEK_TEST_STABLE_ID is unused and should be removed to avoid dead test setup.

#ai-review-inline

github-actions · 2026-09-05T14:30:54Z

+        # Upstream call will fail: the fake proxy does not forward to the real
+        # OpenCode API and the API key is synthetic. We don't care about the
+        # final error; only that the fake proxy was contacted first.
+        with pytest.raises(Exception):


Catching Exception can mask unrelated regressions; assert the specific network or client exception expected from the closed tunnel.

#ai-review-inline

github-actions · 2026-09-05T14:30:54Z

+    def handle(self) -> None:
+        trace = self.server.trace  # type: ignore[attr-defined]
+        try:
+            data = self.request.recv(4096)


A single recv call may return a partial HTTP request line, making the proxy trace assertion flaky; read until CRLF instead.

Suggested change

data = self.request.recv(4096)

first_line = self.request.makefile("rb").readline(4097).rstrip(b"\r\n")

#ai-review-inline

github-actions · 2026-09-05T14:30:54Z

+            try:
+                self.request.settimeout(0.5)
+                self.request.recv(4096)
+            except Exception:


Catching every exception silently can hide defects in the fake proxy; restrict suppression to expected socket errors.

Suggested change

except Exception:

except (OSError, TimeoutError):

pass

#ai-review-inline

github-actions · 2026-09-05T14:30:54Z

+        yield {"trace": trace, "url": proxy_url}
+    finally:
+        server.shutdown()
+        server.server_close()


Join the server thread during teardown to prevent it from leaking into subsequent tests.

Suggested change

server.server_close()

server.server_close()

thread.join(timeout=1)

#ai-review-inline

github-actions · 2026-09-05T14:31:09Z

The change correctly propagates the proxy-aware HTTP client into custom provider plugins for both regular and streaming requests, with useful end-to-end regression coverage. However, resolved_spec=None is indistinguishable from “not supplied,” so an explicitly resolved direct/no-proxy route is resolved again in ProxiedClientPool.get_client(), potentially causing the selected client to disagree with logged metadata; use a sentinel or explicit resolution flag. Adding streaming and direct-route regression tests would further strengthen coverage.

#ai-review-summary

kevincojean added 2 commits September 3, 2026 12:18

fix(proxy): route provider plugins with custom logic through the conf…

a6fe332

…igured proxy

kevincojean marked this pull request as ready for review September 3, 2026 10:02

Copilot AI lite review requested due to automatic review settings September 3, 2026 10:02

Copilot started reviewing on behalf of kevincojean September 3, 2026 10:03 View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

Comment thread src/rotator_library/client/executor.py

Comment thread tests/test_proxy_request_routing.py

Comment thread tests/test_proxy_request_routing.py

build(pyproject): add pytest and pytest-asyncio dependencies

6523780

chore(client/executor): update _resolve_http_client to accept resolved_spec refactor(client/executor): reuse proxy_spec in _

kevincojean force-pushed the fix/proxy-support branch from b911f44 to 6523780 Compare September 3, 2026 10:51

Merge branch 'dev' into fix/proxy-support

4357ce9

b3nw added 2 commits September 5, 2026 09:20

Merge branch 'dev' into fix/proxy-support

81133ea

Merge branch 'dev' into fix/proxy-support

76374ef

github-actions Bot reviewed Sep 5, 2026

View reviewed changes

	!tests/test_proxy_request_routing.py
	!tests/test_proxy_request_routing.py

	resolved_spec: Optional[ProxySpec] = None,
	resolved_spec: Optional[ProxySpec] = None,



		DEEPSEEK_TEST_API_KEY = "sk-fake-deepseek-key-for-proxy-routing-0000"
		DEEPSEEK_TEST_STABLE_ID = hashlib.sha256(DEEPSEEK_TEST_API_KEY.encode()).hexdigest()[:12]

	data = self.request.recv(4096)
	first_line = self.request.makefile("rb").readline(4097).rstrip(b"\r\n")

	server.server_close()
	server.server_close()
	thread.join(timeout=1)

Conversation

kevincojean commented Sep 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

socket-security Bot commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

socket-security Bot commented Sep 3, 2026 •

edited

Loading