Skip to content

refactor(api-proxy): split proxy-request.js into http-client.js and body-handler.js#4939

Merged
lpcox merged 3 commits into
mainfrom
copilot/refactor-split-proxy-request
Jun 14, 2026
Merged

refactor(api-proxy): split proxy-request.js into http-client.js and body-handler.js#4939
lpcox merged 3 commits into
mainfrom
copilot/refactor-split-proxy-request

Conversation

Copilot AI commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

proxy-request.js had grown to 763 lines mixing HTTP client infrastructure, body I/O, transform pipeline, and retry timing — with model-discovery.js reaching back into it just to borrow proxyAgent.

New modules

http-client.js — singleton HTTPS_PROXY / proxyAgent constants

  • Removes the awkward reverse-dependency from model-discovery.js into the proxy core

body-handler.js — body I/O and transform pipeline

  • createBodyHandler({ handleRequestError, otel }) factory — dependency injection avoids the circular import that would arise from collectRequestBody calling handleRequestError
  • collectRequestBody — stream reader with 10 MB enforcement
  • transformRequestBody — pipeline: caller transform → null-tool-call sanitisation → steering injection → stream_options injection
  • sleep indirection + _setSleepForTests/_resetSleepForTestsproxy-request.js calls sleep() for retry backoff so test overrides propagate correctly

Updated files

proxy-request.js (763 → 608 lines)

// Before — everything defined locally
const HTTPS_PROXY = process.env.HTTPS_PROXY || process.env.HTTP_PROXY;
const proxyAgent  = HTTPS_PROXY ? new HttpsProxyAgent(HTTPS_PROXY) : undefined;
// ... 130+ lines of collectRequestBody + transformRequestBody inline ...

// After — wired via imports and factory
const { HTTPS_PROXY, proxyAgent } = require('./http-client');
const { createBodyHandler, sleep, _setSleepForTests, _resetSleepForTests } = require('./body-handler');
// ...
const { collectRequestBody, transformRequestBody } = createBodyHandler({ handleRequestError, otel });

All existing module.exports symbols are preserved for full backwards compatibility.

model-discovery.jsproxyAgent now imported from ./http-client instead of ./proxy-request.

- Extract HTTPS proxy agent into containers/api-proxy/http-client.js
  so model-discovery.js no longer cross-imports from the proxy core.
- Extract collectRequestBody, transformRequestBody, and the _sleep
  indirection into containers/api-proxy/body-handler.js via a
  createBodyHandler(deps) factory, enabling isolated unit testing.
- proxy-request.js imports from both new modules and re-exports all
  previous symbols for full backwards compatibility.
- model-discovery.js now imports proxyAgent from http-client.js.

Closes #4932
Copilot AI changed the title [WIP] Refactor to split proxy request file into focused modules refactor(api-proxy): split proxy-request.js into http-client.js and body-handler.js Jun 14, 2026
Copilot finished work on behalf of lpcox June 14, 2026 17:06
Copilot AI requested a review from lpcox June 14, 2026 17:06
@lpcox lpcox marked this pull request as ready for review June 14, 2026 17:08
Copilot AI review requested due to automatic review settings June 14, 2026 17:08
@github-actions

github-actions Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 96.60% 96.88% 📈 +0.28%
Statements 96.47% 96.75% 📈 +0.28%
Functions 98.80% 98.80% ➡️ +0.00%
Branches 91.18% 91.28% 📈 +0.10%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/workdir-setup.ts 92.6% → 94.4% (+1.85%) 92.6% → 94.4% (+1.85%)
src/artifact-preservation.ts 85.4% → 100.0% (+14.64%) 85.4% → 100.0% (+14.64%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the API proxy core by extracting shared HTTP client infrastructure and request body handling logic out of proxy-request.js, reducing module coupling and removing the previous reverse-dependency where model-discovery.js imported proxyAgent from the proxy core.

Changes:

  • Introduces http-client.js to centralize HTTPS_PROXY and proxyAgent as shared singletons.
  • Introduces body-handler.js to encapsulate request body collection + transform pipeline, using a factory for dependency injection (avoids circular imports) and exporting sleep + test overrides for retry backoff.
  • Updates proxy-request.js and model-discovery.js to import the new modules while preserving proxy-request.js’s existing public exports for compatibility.
Show a summary per file
File Description
containers/api-proxy/proxy-request.js Wires proxy core to new http-client and body-handler modules; keeps existing exported surface while shrinking file size.
containers/api-proxy/model-discovery.js Switches proxyAgent import to http-client to avoid depending on proxy core module initialization.
containers/api-proxy/http-client.js New shared module exporting HTTPS_PROXY and proxyAgent singleton for outbound HTTPS usage.
containers/api-proxy/body-handler.js New module implementing body read + transform pipeline and a test-overridable sleep used by retry backoff.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 4/4 changed files
  • Comments generated: 0

These new modules were split from proxy-request.js but not added to the
Dockerfile COPY directive, causing 'Cannot find module ./http-client'
at container startup.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: Copilot BYOK (Direct) Mode ✅ PASS

Running in direct BYOK mode via COPILOT_PROVIDER_API_KEY (api-proxy → api.githubcopilot.com)

  • ✅ GitHub MCP testing: PR list verified
  • ✅ GitHub.com connectivity: HTTP working
  • ✅ File write/read: Test file confirmed
  • ✅ BYOK inference: Agent executing via sidecar

All tests passed. @lpcox

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions

Copy link
Copy Markdown
Contributor

🔥 Smoke Test Results — Auth mode: PAT (COPILOT_GITHUB_TOKEN)

Test Result
GitHub MCP connectivity ✅ Connected (PR data fetched)
GitHub.com HTTP ✅ HTTP 200
File write/read ⚠️ Pre-step vars not injected (skipped)

Overall: PASS (2/2 verifiable tests passed)

PR: "refactor(api-proxy): split proxy-request.js into http-client.js and body-handler.js"
Author: @Copilot · Assignees: @lpcox @Copilot

🔑 PAT report filed by Smoke Copilot PAT

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smoke Test: API Proxy OpenTelemetry Tracing

Scenario Status Details
1. Module Loading otel.js loads successfully. Exports: startRequestSpan, setTokenAttributes, setBudgetAttributes, endSpan, endSpanError, shutdown, isEnabled
2. Test Suite 59 tests passed, 0 failed across otel.test.js + otel-fanout.test.js
3. Env Var Forwarding OTEL_* vars auto-forwarded via observability-environment.ts (buildOtelEnvironment); specific vars (OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS, GITHUB_AW_OTEL_TRACE_ID, GITHUB_AW_OTEL_PARENT_SPAN_ID) explicitly passed to api-proxy in api-proxy-service-config.ts
4. Token Tracker Integration onUsage callback present in token-tracker-http.js (lines 64, 256) as the OTEL hook point
5. OTEL Diagnostics ⚠️ Live run in progress — cannot confirm span export. otel.js falls back to /var/log/api-proxy/otel.jsonl when no OTLP endpoint configured

Overall: All statically-verifiable scenarios pass. ✅

📡 OTel tracing validated by Smoke OTel Tracing

@github-actions

Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results

PR: refactor(api-proxy): split proxy-request.js into http-client.js and body-handler.js
Author: @Copilot | Assignees: @lpcox, @Copilot

Test Result
GitHub MCP connectivity
GitHub.com HTTP ❌ (pre-step data not injected)
File write/read ❌ (pre-step data not injected)

Overall: FAIL — pre-step template variables were not resolved (${{ steps.smoke-data.outputs.* }})

📰 BREAKING: Report filed by Smoke Copilot

@github-actions

Copy link
Copy Markdown
Contributor

@lpcox @Copilot

  • GitHub MCP connectivity: ✅
  • GitHub.com connectivity: ✅
  • File write/read test: ✅
  • BYOK inference test: ✅
    Running in direct BYOK mode (AWF_AUTH_TYPE=github-oidc + AWF_AUTH_AZURE_* + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) authenticated via Microsoft Entra
    Overall: PASS

🪪 BYOK (AOAI Entra) report filed by Smoke Copilot BYOK AOAI (Entra)

@github-actions

Copy link
Copy Markdown
Contributor

Chroot Version Comparison Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3 ❌ No
Node.js v24.16.0 v22.22.3 ❌ No
Go go1.22.12 go1.22.12 ✅ Yes

Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot

@github-actions

Copy link
Copy Markdown
Contributor

Smoke test results

  • Refactor OpenAI BYOK base URL parsing to reuse shared proxy URL normalization: ✅
  • refactor(model-resolver): decompose resolveModel into focused sub-functions; move version utils tests: ✅
  • GitHub title check: ✅
  • Temp file check: ✅
  • Build check: ✅
    Overall: PASS

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • registry.npmjs.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "registry.npmjs.org"

See Network Configuration for more information.

🔮 The oracle has spoken through Smoke Codex

@github-actions

Copy link
Copy Markdown
Contributor

@Copilot @lpcox

smoke-copilot-byok-aoai-apikey results:

  • GitHub API: ✅
  • github.com HTTP: ✅
  • File I/O: ✅
  • BYOK Inference: ✅

Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw)

Overall: PASS

🔑 BYOK (AOAI api-key) report filed by Smoke Copilot BYOK AOAI (api-key)

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test Results

Overall Status: FAIL

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • localhost

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "localhost"

See Network Configuration for more information.

💎 Faceted by Smoke Gemini

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: GitHub Actions Services Connectivity

Check Result
Redis PING (host.docker.internal:6379) ❌ Timeout
PostgreSQL pg_isready (host.docker.internal:5432) ❌ No response
PostgreSQL SELECT 1 ❌ Not reached

Overall: FAIL

host.docker.internal resolves to 172.17.0.1, but TCP connections to both ports timed out. The AWF firewall blocks database ports (Redis 6379, PostgreSQL 5432) by design per setup-iptables.sh.

🔌 Service connectivity validated by Smoke Services

@github-actions

Copy link
Copy Markdown
Contributor

🏗️ Build Test Suite Results

Ecosystem Project Build/Install Tests Status
Bun elysia 1/1 passed ✅ PASS
Bun hono 1/1 passed ✅ PASS
C++ fmt N/A ✅ PASS
C++ json N/A ✅ PASS
Deno oak N/A 1/1 passed ✅ PASS
Deno std N/A 1/1 passed ✅ PASS
.NET hello-world N/A ✅ PASS
.NET json-parse N/A ✅ PASS
Go color 1/1 passed ✅ PASS
Go env 1/1 passed ✅ PASS
Go uuid 1/1 passed ✅ PASS
Java gson 1/1 passed ✅ PASS
Java caffeine 1/1 passed ✅ PASS
Node.js clsx all passed ✅ PASS
Node.js execa all passed ✅ PASS
Node.js p-limit all passed ✅ PASS
Rust fd 1/1 passed ✅ PASS
Rust zoxide 1/1 passed ✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS

Generated by Build Test Suite for issue #4939 ·

@lpcox lpcox merged commit a9399ec into main Jun 14, 2026
81 of 84 checks passed
@lpcox lpcox deleted the copilot/refactor-split-proxy-request branch June 14, 2026 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants