Skip to content

Refactor api-proxy startup to adapter-only validation/model discovery paths#3444

Merged
lpcox merged 2 commits into
mainfrom
copilot/fix-duplicate-code-legacy-paths
May 20, 2026
Merged

Refactor api-proxy startup to adapter-only validation/model discovery paths#3444
lpcox merged 2 commits into
mainfrom
copilot/fix-duplicate-code-legacy-paths

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 20, 2026

validateApiKeys() and fetchStartupModels() had parallel “legacy override” branches that duplicated provider-specific URL/auth/header logic already encapsulated by adapters. This created drift risk across startup validation, startup model fetch, and provider adapters.

  • What changed

    • Removed legacy override-object branches from:
      • containers/api-proxy/server.js#validateApiKeys
      • containers/api-proxy/server.js#fetchStartupModels
    • Simplified both function signatures to adapter-array inputs:
      • validateApiKeys(adapters = [])
      • fetchStartupModels(adapters = [])
    • Kept behavior delegated to adapter contracts:
      • getValidationProbe()
      • getModelsFetchConfig()
  • Test updates

    • Migrated tests that passed override objects to explicit mock adapters:
      • containers/api-proxy/server.billing.test.js now builds probe adapters for validateApiKeys.
      • containers/api-proxy/server.network.test.js now builds model-fetch adapters for fetchStartupModels.
      • containers/api-proxy/server.lifecycle.test.js uses fetchStartupModels([]) for no-provider completion checks.
  • Why this reduces maintenance risk

    • Provider auth/target/header logic now lives in one place (adapter implementations), eliminating duplicated startup logic that needed parallel updates for every provider change.
// before: test compatibility override object
await validateApiKeys({ openaiKey: 'sk-test', openaiTarget: 'api.openai.com' });

// after: adapter-driven probe (same production contract)
await validateApiKeys([
  {
    name: 'openai',
    getValidationProbe: () => ({
      url: 'https://api.openai.com/v1/models',
      opts: { method: 'GET', headers: { Authorization: 'Bearer sk-test' } },
    }),
  },
]);

Copilot AI changed the title [WIP] Refactor duplicate legacy paths in validateApiKeys and fetchStartupModels Refactor api-proxy startup to adapter-only validation/model discovery paths May 20, 2026
Copilot finished work on behalf of lpcox May 20, 2026 00:43
Copilot AI requested a review from lpcox May 20, 2026 00:43
@lpcox lpcox marked this pull request as ready for review May 20, 2026 01:50
Copilot AI review requested due to automatic review settings May 20, 2026 01:50
@github-actions
Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 95.53% 95.60% 📈 +0.07%
Statements 95.37% 95.44% 📈 +0.07%
Functions 96.41% 96.41% ➡️ +0.00%
Branches 87.91% 87.95% 📈 +0.04%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/config-writer.ts 83.0% → 85.6% (+2.54%) 83.0% → 85.6% (+2.54%)

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

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results

✅ GitHub API: 2 PR entries found in recent-prs.json
✅ Playwright: GitHub.com title verified
✅ File verify: smoke-test file exists

Result: PASS — All smoke tests passed.

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results

Test Result
GitHub MCP connectivity ✅ PR data fetched successfully
GitHub.com HTTP connectivity ❌ Pre-step data not available (template vars unexpanded)
File write/read ❌ Pre-step data not available (template vars unexpanded)

PR: "Refactor api-proxy startup to adapter-only validation/model discovery paths"
Author: @Copilot | Assignees: @lpcox, @Copilot

Overall: FAIL — pre-computed test data was not injected (workflow template variables were not expanded before agent invocation).

📰 BREAKING: Report filed by Smoke Copilot

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 startup flows so API key validation and startup model discovery rely exclusively on provider adapter contracts (getValidationProbe() / getModelsFetchConfig()), removing legacy “override object” branches that duplicated provider-specific URL/auth/header logic.

Changes:

  • Simplified validateApiKeys and fetchStartupModels to accept ProviderAdapter[] only (defaulting to []).
  • Removed inline legacy override-based probe/fetch logic from containers/api-proxy/server.js.
  • Updated server tests to pass explicit mock adapters (or []) instead of override objects.
Show a summary per file
File Description
containers/api-proxy/server.js Removes legacy override branches and makes startup validation/model-fetch purely adapter-driven.
containers/api-proxy/server.billing.test.js Updates key-validation tests to use mock validation adapters instead of override objects.
containers/api-proxy/server.network.test.js Updates startup model-fetch tests to use mock model-fetch adapters; keeps a real Copilot adapter case.
containers/api-proxy/server.lifecycle.test.js Updates lifecycle health tests to call fetchStartupModels([]) for “no providers” completion.

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

@github-actions
Copy link
Copy Markdown
Contributor

🔥 Smoke Test: Copilot BYOK Offline Mode

Test Result
GitHub MCP connectivity
GitHub.com HTTP ⚠️ (template vars unresolved — pre-step skipped)
File write/read ⚠️ (template vars unresolved — pre-step skipped)
BYOK inference (agent → api-proxy → api.githubcopilot.com)

Running in BYOK offline mode (COPILOT_OFFLINE=true) via api-proxy → api.githubcopilot.com.

PR by @Copilot, assignees: @lpcox, @Copilot.

Overall: PARTIAL — BYOK inference path confirmed working; pre-step data was not injected (template vars unresolved).

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

PRs: Refactor option parser facade to direct re-exports and unify flag validation type; tests: replace host-env barrel imports with canonical host-env split modules
✅ GitHub PR review; ❌ safeinputs-gh unavailable; ✅ Playwright title
❌ Tavily no tools; ✅ file write/bash; ❌ discussion query unavailable; ✅ build
Overall status: FAIL

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

Gemini Engine Smoke Test Results

  • GitHub MCP Testing: ❌ (Tools missing)
  • GitHub.com Connectivity: ❌ (Status 000/400, SSL Error 35)
  • 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

Chroot Smoke Test Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3 ❌ NO
Node.js v24.15.0 v20.20.2 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

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

Tested by Smoke Chroot

@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 #3444 · ● 6.4M ·

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results

Check Result
Redis PING ❌ No response (timeout)
PostgreSQL pg_isready no response
PostgreSQL SELECT 1 ❌ Connection refused

Overall: FAIL — Service containers not reachable via host.docker.internal or 127.0.0.1.

🔌 Service connectivity validated by Smoke Services

@lpcox lpcox merged commit 1689309 into main May 20, 2026
68 of 74 checks passed
@lpcox lpcox deleted the copilot/fix-duplicate-code-legacy-paths branch May 20, 2026 03:37
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.

[Duplicate Code] Legacy override paths in validateApiKeys() and fetchStartupModels() duplicate adapter encapsulation

3 participants