Skip to content

refactor: split buildApiProxyBaseEnv into focused env-var builders#6033

Merged
lpcox merged 3 commits into
mainfrom
copilot/refactor-split-build-api-proxy
Jul 9, 2026
Merged

refactor: split buildApiProxyBaseEnv into focused env-var builders#6033
lpcox merged 3 commits into
mainfrom
copilot/refactor-split-build-api-proxy

Conversation

Copilot AI commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

buildApiProxyBaseEnv() in src/services/api-proxy-env-config.ts was a 157-line function handling 10+ unrelated concern groups — a security-critical path where credential flow should be auditable in isolation.

Changes

src/services/api-proxy-env-config.ts — extracted seven focused sub-functions, all @internal-exported for testability:

  • buildCredentialEnv(config) — API keys for OpenAI, Anthropic, Copilot, Gemini, Vertex
  • buildProviderRoutingEnv(config) — provider targets, GitHub enterprise URLs, platform type, integration identity
  • buildProxyRoutingEnv(networkConfig) — Squid HTTP_PROXY / HTTPS_PROXY / NO_PROXY
  • buildOtelEnv() — OTLP endpoint, headers, service name, trace context forwarding
  • buildRateLimitEnv(config)AWF_RATE_LIMIT_*, token guards, run limits, agent timeout
  • buildModelPolicyEnv(config) — model aliases, allowed/disallowed models, Anthropic cache, token steering, diagnostics
  • buildOidcEnv(config) — OIDC provider vars, GitHub Actions runtime tokens, custom auth headers

buildApiProxyBaseEnv is now a thin orchestrator:

export function buildApiProxyBaseEnv(config: WrapperConfig, networkConfig: NetworkConfig): Record<string, string> {
  return {
    ...buildCredentialEnv(config),
    ...buildProviderRoutingEnv(config),
    ...buildProxyRoutingEnv(networkConfig),
    ...buildOtelEnv(),
    ...buildRateLimitEnv(config),
    ...buildModelPolicyEnv(config),
    ...buildOidcEnv(config),
  };
}

Public API surface is unchanged — the sole external caller (api-proxy-service-config.ts) required no modification.

src/services/api-proxy-env-config.test.ts (new) — 49 unit tests covering each sub-function in isolation (credential isolation, proxy routing, OIDC conditional forwarding, rate limit omission, etc.).

Extract seven focused sub-functions from the 157-line buildApiProxyBaseEnv monolith in
src/services/api-proxy-env-config.ts, each covering one distinct concern group:

- buildCredentialEnv(config)      — API keys (OpenAI, Anthropic, Copilot, Gemini, Vertex)
- buildProviderRoutingEnv(config) — provider targets, GitHub enterprise URLs, platform type,
                                    integration identity
- buildProxyRoutingEnv(networkConfig) — Squid HTTP_PROXY/HTTPS_PROXY/NO_PROXY
- buildOtelEnv()                  — OTEL endpoint, headers, service name, trace context
- buildRateLimitEnv(config)       — AWF_RATE_LIMIT_* vars, token guards, run limits
- buildModelPolicyEnv(config)     — model aliases, allowed/disallowed models, Anthropic cache,
                                    token steering, diagnostic logging
- buildOidcEnv(config)            — OIDC provider vars, GitHub Actions OIDC tokens,
                                    custom auth headers

buildApiProxyBaseEnv is now a thin orchestrator that composes all seven builders.
All sub-functions are exported with @internal JSDoc and ts-prune-ignore-next for unit testing.

Add api-proxy-env-config.test.ts with 49 unit tests covering each sub-function in isolation.

Closes #6010
Copilot AI changed the title [WIP] Refactor buildApiProxyBaseEnv into focused environment variable builders refactor: split buildApiProxyBaseEnv into focused env-var builders Jul 9, 2026
Copilot AI requested a review from lpcox July 9, 2026 02:50
Copilot finished work on behalf of lpcox July 9, 2026 02:50
@lpcox lpcox marked this pull request as ready for review July 9, 2026 02:53
Copilot AI review requested due to automatic review settings July 9, 2026 02:53

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 buildApiProxyBaseEnv() (a security-sensitive env assembly path for the api-proxy sidecar) by splitting it into smaller, concern-focused env-var builder functions and adds a new Jest test suite to validate each builder in isolation.

Changes:

  • Split the previous monolithic env assembly into multiple focused builders (credentials, provider routing, proxy routing, OTEL, rate-limits, model policy, OIDC) and made buildApiProxyBaseEnv() a thin orchestrator.
  • Added a new unit test file covering the new builders and the orchestrator composition.
Show a summary per file
File Description
src/services/api-proxy-env-config.ts Refactors env construction into multiple smaller builder functions and composes them via a thin orchestrator.
src/services/api-proxy-env-config.test.ts Adds new unit tests covering each builder and the orchestrator output.

Review details

Tip

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

  • Files reviewed: 2/2 changed files
  • Comments generated: 3
  • Review effort level: Low

Comment thread src/services/api-proxy-env-config.ts Outdated
Comment on lines +72 to +76
* Builds API credential environment variables for the api-proxy sidecar.
* These keys are passed securely to the sidecar and are NOT visible to the agent container.
* @internal Exported for testing
*/
export function buildCredentialEnv(config: WrapperConfig): Record<string, string> {
Comment thread src/services/api-proxy-env-config.ts Outdated
Comment on lines +262 to +264
export function buildOidcEnv(config: WrapperConfig): Record<string, string> {
const normalizedAuthType = (config.authType?.toLowerCase().trim()) || getLowerCaseProcessEnvValue('AWF_AUTH_TYPE') || '';

Comment on lines +382 to +386
it('sets AWF_AUTH_ANTHROPIC_TOKEN_URL when anthropicTokenUrl is configured', () => {
const env = buildOidcEnv({ ...baseConfig, workDir: '/tmp/awf-test', anthropicTokenUrl: 'https://auth.anthropic.com/token' });
expect(env.AWF_AUTH_ANTHROPIC_TOKEN_URL).toBe('https://auth.anthropic.com/token');
});
});
@lpcox

lpcox commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

@copilot address review feedback

Copilot AI commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

@copilot address review feedback

Addressed all three review points in commit refactor: testHelpers convention and fix AWF_AUTH_TYPE resolution:

  1. Export convention — Removed individual export keywords from the seven helper functions; they're now internal. Added export const testHelpers = { ... } with @internal / // ts-prune-ignore-next at the bottom of the file, following the pattern in agent-service.ts and optional-services.ts.

  2. normalizedAuthType resolution — Updated buildOidcEnv to consult getConfigEnvValue(config, 'AWF_AUTH_TYPE') (which covers additionalEnv and envFile) before falling back to getLowerCaseProcessEnvValue, so --env AWF_AUTH_TYPE=github-oidc now correctly enables ACTIONS token forwarding.

  3. New test — Added 'forwards ACTIONS_ID_TOKEN_REQUEST_* when AWF_AUTH_TYPE is set via additionalEnv' to the buildOidcEnv suite. All 50 tests pass.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Smoke Copilot BYOK AOAI (api-key) reports failed. AOAI BYOK (api-key) mode investigation needed...

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Smoke Claude passed

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation...

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Build Test Suite completed successfully!

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🔌 Smoke Services — All services reachable! ✅

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Smoke Gemini completed. All facets verified. 💎

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🚀 Security Guard has started processing this pull request

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Contribution Check completed successfully!

PR #6033 follows the applicable CONTRIBUTING.md guidelines: it has a clear description, adds focused tests for the refactor, keeps changes in src/services, and no documentation update appears necessary for the internal API-preserving change.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 98.98% 98.98% ➡️ +0.00%
Statements 98.94% 98.95% 📈 +0.01%
Functions 99.44% 99.45% 📈 +0.01%
Branches 95.64% 95.67% 📈 +0.03%

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

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Smoke Test: Claude Engine Validation

Check Result
API Status ✅ PASS
GH Check ✅ PASS
File Status ✅ PASS

Overall Result: PASS

Warning

Firewall blocked 1 domain

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

  • awmgmcpg

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

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

Generated by Smoke Claude for #6033 · 55.2 AIC · ⊞ 3.3K ·
Add label ready-for-aw to run again

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Smoke Test: GitHub Actions Services Connectivity

  • Redis PING: ❌ (Network is unreachable)
  • PostgreSQL pg_isready: ❌ (no response)
  • PostgreSQL SELECT 1: ❌ (Network is unreachable)

Overall: FAILhost.docker.internal (172.17.0.1) is unreachable from this runner. Service containers do not appear to be running or accessible.

Warning

Firewall blocked 1 domain

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

  • awmgmcpg

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

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

🔌 Service connectivity validated by Smoke Services
Add label ready-for-aw to run again

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Smoke Test: Gemini Engine Validation

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
Add label ready-for-aw to run again

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Smoke Test Results

Test Result
GitHub MCP ❌ (not available in agent context)
GitHub.com HTTP ❓ (pre-step data unavailable)
File Write/Read ❓ (pre-step data unavailable)

Overall: ⚠️ INCONCLUSIVE — pre-step outputs (${{ steps.smoke-data.outputs.* }}) were not expanded before agent invocation.

cc @lpcox

Warning

Firewall blocked 1 domain

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

  • awmgmcpg

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

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

📰 BREAKING: Report filed by Smoke Copilot
Add label ready-for-aw to run again

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🔬 Smoke Test: Copilot PAT Auth

Test Result
GitHub MCP connectivity
GitHub.com HTTP ✅ 200
File write/read

Overall: PASS 🎉
Auth mode: PAT (COPILOT_GITHUB_TOKEN) · Author: @lpcox

Warning

Firewall blocked 1 domain

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

  • awmgmcpg

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

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

🔑 PAT report filed by Smoke Copilot PAT
Add label ready-for-aw to run again

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

GitHub MCP tool: ✅
GitHub.com connectivity: ✅
File write/read: ✅
BYOK inference: ✅
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
@lpcox

Warning

Firewall blocked 1 domain

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

  • awmgmcpg

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

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

🪪 BYOK (AOAI Entra) report filed by Smoke Copilot BYOK AOAI (Entra)
Add label ready-for-aw to run again

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🔄 Smoke Test Results: Copilot BYOK (Direct) Mode

Test Result
GitHub MCP Connectivity
GitHub.com HTTP Code
File Write/Read
BYOK Inference Path

Status: PASS

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

Warning

Firewall blocked 1 domain

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

  • awmgmcpg

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

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

🔑 BYOK report filed by Smoke Copilot BYOK
Add label ready-for-aw to run again

@github-actions

github-actions Bot commented Jul 9, 2026

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.18.0 v22.23.1 ❌ 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.

Warning

Firewall blocked 1 domain

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

  • awmgmcpg

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

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

Tested by Smoke Chroot
Add label ready-for-aw to run again

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Smoke Test: API Proxy OpenTelemetry Tracing

Scenario Result Notes
1. Module Loading ✅ Pass otel.js loads successfully; exports: startRequestSpan, setTokenAttributes, setBudgetAttributes, endSpan, endSpanError, shutdown, isEnabled
2. Test Suite ✅ Pass 39/39 tests passed — module init, span creation, token attributes, budget attributes, span lifecycle, OTLP serialization, exporter classes
3. Env Var Forwarding ✅ Pass src/services/api-proxy-env-config.ts::buildOtelEnv() forwards GH_AW_OTLP_ENDPOINTS, OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS, GITHUB_AW_OTEL_TRACE_ID, GITHUB_AW_OTEL_PARENT_SPAN_ID, OTEL_SERVICE_NAME
4. Token Tracker Integration ✅ Pass onUsage callback exists in token-tracker-http.js (line 343) as the OTEL hook point
5. OTEL Diagnostics i️ N/A No live run artifacts available; fallback to FileSpanExporter (/var/log/api-proxy/otel.jsonl) when no OTLP endpoint is set

All scenarios pass. OTEL tracing integration is fully functional — fan-out export via GH_AW_OTLP_ENDPOINTS, single-endpoint fallback via OTEL_EXPORTER_OTLP_ENDPOINT, and graceful file-based degradation when neither is configured.

Warning

Firewall blocked 1 domain

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

  • awmgmcpg

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

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

📡 OTel tracing validated by Smoke OTel Tracing
Add label ready-for-aw to run again

@github-actions github-actions Bot mentioned this pull request Jul 9, 2026
@github-actions

github-actions Bot commented Jul 9, 2026

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 passed ✅ PASS
Go env passed ✅ PASS
Go uuid passed ✅ PASS
Java gson 1/1 passed ✅ PASS
Java caffeine 1/1 passed ✅ PASS
Node.js clsx passed ✅ PASS
Node.js execa passed ✅ PASS
Node.js p-limit passed ✅ PASS
Rust fd 1/1 passed ✅ PASS
Rust zoxide 1/1 passed ✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS

Warning

Firewall blocked 1 domain

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

  • awmgmcpg

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

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

Generated by Build Test Suite for #6033 · 47.2 AIC · ⊞ 6.9K ·
Add label ready-for-aw to run again

@lpcox lpcox merged commit d218413 into main Jul 9, 2026
86 of 88 checks passed
@lpcox lpcox deleted the copilot/refactor-split-build-api-proxy branch July 9, 2026 03:28
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.

[Refactoring] Split buildApiProxyBaseEnv() into focused env-var builders

3 participants