Skip to content

Refactor OpenAI BYOK base URL parsing to reuse shared proxy URL normalization#4949

Merged
lpcox merged 3 commits into
mainfrom
copilot/fix-duplicate-url-normalisation
Jun 14, 2026
Merged

Refactor OpenAI BYOK base URL parsing to reuse shared proxy URL normalization#4949
lpcox merged 3 commits into
mainfrom
copilot/fix-duplicate-url-normalisation

Conversation

Copilot AI commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

providers/openai.js had a local parseByokBaseUrl() that duplicated the core URL normalization logic already implemented in proxy-utils.js (scheme detection, implicit https://, URL parse/hostname extraction). This centralizes that parsing path to avoid drift while preserving BYOK-specific path handling.

  • Shared parsing primitive in proxy-utils

    • Added parseApiTargetAndBasePath(value) to return { target, basePath } from a single parse pass.
    • Reused the same normalization core as normalizeApiTarget via an internal shared parser.
  • OpenAI provider now consumes shared utility

    • Removed provider-local parseByokBaseUrl() from providers/openai.js.
    • Switched BYOK base URL handling to parseApiTargetAndBasePath(env.COPILOT_PROVIDER_BASE_URL).
  • Behavioral intent preserved

    • normalizeApiTarget retains warning behavior for unsupported URL components.
    • BYOK parsing continues to accept path components (e.g., Azure deployment paths) without introducing those warnings.
// providers/openai.js
const { parseApiTargetAndBasePath, validateAuthHeaderEnv } = require('../proxy-utils');

const { target: copilotByokTarget, basePath: copilotByokBasePath } =
  parseApiTargetAndBasePath(env.COPILOT_PROVIDER_BASE_URL);

Copilot AI changed the title [WIP] Fix duplicate URL normalisation in openai.js Refactor OpenAI BYOK base URL parsing to reuse shared proxy URL normalization Jun 14, 2026
Copilot finished work on behalf of lpcox June 14, 2026 17:39
Copilot AI requested a review from lpcox June 14, 2026 17:39
@lpcox lpcox marked this pull request as ready for review June 14, 2026 17:40
Copilot AI review requested due to automatic review settings June 14, 2026 17:40

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 pull request refactors OpenAI Azure BYOK base URL parsing in the API proxy to reuse shared URL normalization/parsing logic in proxy-utils.js, reducing duplicated parsing behavior between providers and core utilities.

Changes:

  • Introduces parseApiTargetAndBasePath() in containers/api-proxy/proxy-utils.js built on a shared internal URL parsing helper.
  • Updates the OpenAI provider to use the new shared parser instead of a provider-local parseByokBaseUrl() implementation.
  • Adds/extends unit tests to cover the new shared parsing behavior and to ensure BYOK base URLs with path components do not trigger console.warn.
Show a summary per file
File Description
containers/api-proxy/server.routing.test.js Adds unit coverage for parseApiTargetAndBasePath() behavior.
containers/api-proxy/proxy-utils.js Refactors normalizeApiTarget() to use a shared parser; adds parseApiTargetAndBasePath() and internal parseApiTargetUrl().
containers/api-proxy/providers/openai.js Removes provider-local BYOK URL parsing and switches to shared parseApiTargetAndBasePath().
containers/api-proxy/oidc-token-provider.test.js Adds a test ensuring BYOK base URLs with path/query don’t produce warnings during OpenAI adapter creation.

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: 2

@@ -23,29 +23,70 @@ const { URL } = require('url');
function normalizeApiTarget(value) {
Comment thread containers/api-proxy/oidc-token-provider.test.js Outdated
- JSDoc now documents that falsy values (including '') are returned as-is
- Test name clarifies it covers path + query string, not just path

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

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

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: Copilot BYOK (Direct) Mode ✅

All tests passed

Mode: Direct BYOK (COPILOT_PROVIDER_API_KEY via api-proxy sidecar)

Status: PASS

@lpcox @Copilot

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smoke Test: API Proxy OpenTelemetry Tracing

Scenario Result Notes
1. Module Loading ✅ Pass otel.js loads; exports startRequestSpan, setTokenAttributes, setBudgetAttributes, endSpan, endSpanError, shutdown, isEnabled + test helpers
2. Test Suite ✅ Pass otel.test.js exists with full coverage using InMemorySpanExporter; tests span attributes, parent context, status codes, and graceful degradation
3. Env Var Forwarding ✅ Pass api-proxy-service-config.ts forwards GH_AW_OTLP_ENDPOINTS, OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS, GITHUB_AW_OTEL_TRACE_ID, GITHUB_AW_OTEL_PARENT_SPAN_ID via pickEnvVars()
4. Token Tracker Integration ✅ Pass token-tracker-http.js accepts onUsage callback in trackTokenUsage() opts; callback is invoked after normalized usage extraction
5. OTEL Diagnostics ✅ Pass (expected) No OTLP endpoint configured → file fallback active (/var/log/api-proxy/otel.jsonl); no spans exported to remote collector (by design)

All scenarios pass. OTEL tracing integration is complete: spans are created per request, token usage attributes follow GenAI semconv, parent context propagation is wired via env vars, and graceful degradation to file export works when no endpoint is configured.

📡 OTel tracing validated by Smoke OTel Tracing

@github-actions

Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results

PR: Refactor OpenAI BYOK base URL parsing to reuse shared proxy URL normalization
Author: @Copilot | Assignees: @lpcox @Copilot

Test Result
GitHub MCP connectivity
GitHub.com HTTP connectivity ✅ (200)
File write/read ⚠️ pre-step data not expanded

Overall: PASS (2/2 verifiable tests passed; file test skipped due to unexpanded workflow template)

📰 BREAKING: Report filed by Smoke Copilot

@github-actions

Copy link
Copy Markdown
Contributor

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
refactor(token-tracker-http): decompose 238-line trackTokenUsage into testable top-level functions
GitHub title check: ✅
Smoke file: ✅
Build: ✅
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 github-actions Bot mentioned this pull request Jun 14, 2026
@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
Node.js v24.16.0 v22.22.3
Go go1.22.12 go1.22.12

Overall result: ❌ FAILED — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot

@github-actions

Copy link
Copy Markdown
Contributor

@Copilot @lpcox

Smoke Test Results:

  • GitHub MCP: ✅
  • GitHub.com Connectivity: ✅
  • File I/O in Agent: ✅
  • Direct BYOK via api-proxy→Azure (o4-mini-aw): ✅

Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL).
Overall: PASS

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

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: Gemini Engine Validation

✅ GitHub MCP Testing
❌ GitHub.com Connectivity (HTTP 000)
✅ File Writing Testing
✅ Bash Tool Testing

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: PAT Auth — PASS

Auth mode: PAT (COPILOT_GITHUB_TOKEN)
PR: "Refactor OpenAI BYOK base URL parsing to reuse shared proxy URL normalization" — @Copilot
Assignees: @lpcox @Copilot

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

Overall: PASS

🔑 PAT report filed by Smoke Copilot PAT

@github-actions

Copy link
Copy Markdown
Contributor

@lpcox smoke test results:

  • GitHub MCP connectivity: ❌
  • GitHub.com connectivity: ❌
  • File read/write test: ❌
  • Direct 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: FAIL

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

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: GitHub Actions Services Connectivity

Check Result
Redis PING ❌ timeout (no PONG)
PostgreSQL pg_isready ❌ no response
PostgreSQL SELECT 1 ❌ no connection

host.docker.internal resolves to 172.17.0.1 but services are unreachable on ports 6379 and 5432. Service containers may not be running.

Overall: ❌ FAIL

🔌 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 #4949 ·

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