Skip to content

refactor(api-proxy): replace duplicate getUnconfiguredHealthResponse methods with declarative factory params#5926

Merged
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-handle-health-responses
Jul 5, 2026
Merged

refactor(api-proxy): replace duplicate getUnconfiguredHealthResponse methods with declarative factory params#5926
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-handle-health-responses

Conversation

Copilot AI commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Each of the four API proxy provider adapters (anthropic, copilot, gemini, vertex) contained an identical-shaped getUnconfiguredHealthResponse() wrapper that differed only by service name, credential message, and an optional OIDC branch — 20+ duplicate lines across 4 files.

Approach

Three new optional params on buildProviderAdapter replace the imperative methods:

Param Purpose
healthServiceName e.g. 'awf-api-proxy-gemini'
missingCredentialMessage default error text when credentials absent
unavailableWhen () => { message, status? } | null for OIDC states

When healthServiceName + missingCredentialMessage are provided (and no explicit getUnconfiguredHealthResponse is passed), the factory auto-generates the method. An explicit function still takes precedence for escape-hatch use.

Before / After

// Before — repeated in every provider file
getUnconfiguredHealthResponse() {
  if (oidcConfigured) {
    return makeUnconfiguredHealthResponse('awf-api-proxy-copilot', `Copilot OIDC token (${authProvider}) not yet available in api-proxy sidecar`);
  }
  return makeUnconfiguredHealthResponse('awf-api-proxy-copilot', 'COPILOT_GITHUB_TOKEN or COPILOT_PROVIDER_API_KEY not configured in api-proxy sidecar');
},
// After — declarative metadata in buildProviderAdapter call
healthServiceName: 'awf-api-proxy-copilot',
missingCredentialMessage: 'COPILOT_GITHUB_TOKEN or COPILOT_PROVIDER_API_KEY not configured in api-proxy sidecar',
unavailableWhen: () => oidcConfigured ? { message: `Copilot OIDC token (${authProvider}) not yet available in api-proxy sidecar` } : null,

Files changed

  • adapter-factory.js — imports makeUnconfiguredHealthResponse once; adds the three new params and auto-generation logic
  • providers/anthropic.js, copilot.js — OIDC branch expressed via unavailableWhen; makeUnconfiguredHealthResponse import removed
  • providers/gemini.js, vertex.js — reduced to two static strings; makeUnconfiguredHealthResponse import removed
  • adapter-factory.test.js — 4 new tests covering auto-generation, unavailableWhen override, null fallback, and explicit-function precedence
  • ADDING-A-PROVIDER.md — documents the new declarative params with usage tip

…clarative factory params

Replace the four imperative getUnconfiguredHealthResponse() wrappers in
anthropic.js, copilot.js, gemini.js, and vertex.js with three new declarative
params on buildProviderAdapter: healthServiceName, missingCredentialMessage,
and optional unavailableWhen.

The factory now auto-generates getUnconfiguredHealthResponse from those values,
eliminating >20 duplicate lines across 4 copies while keeping provider-specific
wording close to configuration.

Providers that need OIDC-state-dependent messaging (anthropic, copilot) pass an
unavailableWhen callback; simple providers (gemini, vertex) just pass the two
static strings.

Removes makeUnconfiguredHealthResponse imports from all four provider files
(now imported once inside adapter-factory.js).

Adds 4 new unit tests in adapter-factory.test.js covering:
- auto-generation from healthServiceName + missingCredentialMessage
- unavailableWhen override path
- unavailableWhen returning null falls back to missingCredentialMessage
- explicit getUnconfiguredHealthResponse takes precedence over declarative params

Updates ADDING-A-PROVIDER.md to document the new declarative approach.

Closes #5919
Copilot AI changed the title [WIP] Refactor duplicate health response logic in API proxies refactor(api-proxy): replace duplicate getUnconfiguredHealthResponse methods with declarative factory params Jul 5, 2026
Copilot finished work on behalf of lpcox July 5, 2026 12:56
Copilot AI requested a review from lpcox July 5, 2026 12:56
@lpcox lpcox marked this pull request as ready for review July 5, 2026 13:15
Copilot AI review requested due to automatic review settings July 5, 2026 13:15

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

Refactors the api-proxy provider adapters to remove duplicated getUnconfiguredHealthResponse() implementations by letting buildProviderAdapter() auto-generate the /health “not configured” response from declarative metadata.

Changes:

  • Added healthServiceName, missingCredentialMessage, and unavailableWhen inputs to buildProviderAdapter() and auto-generation logic for getUnconfiguredHealthResponse.
  • Updated Anthropic/Copilot to express OIDC “not yet available” health states via unavailableWhen; updated Gemini/Vertex to supply only static declarative strings.
  • Added unit tests for the new auto-generation behavior and documented the new parameters in the provider authoring guide.
Show a summary per file
File Description
containers/api-proxy/adapter-factory.js Adds declarative health metadata params and factory auto-generation of getUnconfiguredHealthResponse.
containers/api-proxy/adapter-factory.test.js Adds coverage for auto-generated health response behavior and precedence rules.
containers/api-proxy/providers/anthropic.js Replaces imperative health wrapper with declarative metadata + unavailableWhen for OIDC.
containers/api-proxy/providers/copilot.js Replaces imperative health wrapper with declarative metadata + unavailableWhen for OIDC.
containers/api-proxy/providers/gemini.js Removes per-provider health helper import; supplies declarative health metadata.
containers/api-proxy/providers/vertex.js Removes per-provider health helper import; supplies declarative health metadata.
containers/api-proxy/providers/ADDING-A-PROVIDER.md Documents the new declarative health parameters for new providers.

Review details

Tip

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

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

Comment on lines +231 to +244
// Auto-generate getUnconfiguredHealthResponse from declarative metadata when
// no explicit function is provided. The optional unavailableWhen callback
// allows providers with OIDC to surface a dynamic message/status.
if (getUnconfiguredHealthResponse === undefined && healthServiceName !== undefined && missingCredentialMessage !== undefined) {
getUnconfiguredHealthResponse = () => {
if (unavailableWhen) {
const override = unavailableWhen();
if (override) {
return makeUnconfiguredHealthResponse(healthServiceName, override.message, override.status);
}
}
return makeUnconfiguredHealthResponse(healthServiceName, missingCredentialMessage);
};
}
| `getUnconfiguredHealthResponse()` | ➖ optional | `/health` response when not enabled — prefer the declarative form below |
| `healthServiceName` | ➖ optional | Service name for auto-generated `/health` response (e.g. `'awf-api-proxy-myprovider'`); requires `missingCredentialMessage` |
| `missingCredentialMessage` | ➖ optional | Default error message when credentials are absent (requires `healthServiceName`) |
| `unavailableWhen` | ➖ optional | `() => { message, status? } \| null` — when non-null the auto-generated `/health` response uses the returned message/status (e.g. for OIDC token-not-yet-available states) |
@lpcox

lpcox commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

@copilot address review feedback

Copilot AI commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

@copilot address review feedback

Addressed the review feedback in commit 12e2ea6.

  • buildProviderAdapter now throws when declarative health metadata is partially specified (unless an explicit getUnconfiguredHealthResponse is provided).
  • Updated adapter-factory tests to cover partial-metadata validation.
  • Fixed the unavailableWhen docs example to use valid JavaScript and removed the misleading inline \| form.

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

✅ Copilot review passed with no inline comments.

@copilot Add the ready-for-aw label to this PR to trigger agentic CI smoke tests.

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Smoke Claude passed

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

🔌 Smoke Services — All services reachable! ✅

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Build Test Suite completed successfully!

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Security Guard failed. Please review the logs for details.

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Chroot tests failed Smoke Chroot failed - See logs for details.

@github-actions

github-actions Bot commented Jul 5, 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 5, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Contribution Check completed successfully!

PR follows the applicable contribution guidelines: clear description, tests included for new factory behavior, documentation updated, and files are organized under containers/api-proxy.

@github-actions

github-actions Bot commented Jul 5, 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 5, 2026

Copy link
Copy Markdown
Contributor

Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Smoke Gemini completed. All facets verified. 💎

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 98.72% 98.76% 📈 +0.04%
Statements 98.64% 98.68% 📈 +0.04%
Functions 99.58% 99.58% ➡️ +0.00%
Branches 94.69% 94.69% ➡️ +0.00%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/workdir-setup.ts 93.2% → 94.9% (+1.69%) 93.2% → 94.9% (+1.69%)

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

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

@lpcox Smoke Test Results:
• GH MCP Testing: ✅
• 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

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 5, 2026

Copy link
Copy Markdown
Contributor

Smoke Test: Copilot BYOK (Direct) Mode

PASS

  • ✅ GitHub.com connectivity (HTTP 200)
  • ✅ File write/read working
  • ✅ Direct BYOK mode active (agent → api-proxy → api.githubcopilot.com)
  • ✅ BYOK inference path functional

Running in direct BYOK mode via COPILOT_PROVIDER_API_KEY with api-proxy sidecar credential injection.

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 5, 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 #5926 · 55.5 AIC · ⊞ 3.3K ·
Add label ready-for-aw to run again

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

🧪 Smoke Test Results

Test Status
GitHub MCP connectivity
GitHub.com HTTP ✅ 200
File write/read ⚠️ (template vars not substituted in workflow)

Overall: PASS

PR 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.

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

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results — PAT Auth

Test Result
GitHub MCP connectivity
GitHub.com HTTP ✅ 200
File write/read ⚠️ template vars unresolved

PR: refactor(api-proxy): replace duplicate getUnconfiguredHealthResponse methods with declarative factory params
Author: @Copilot | Assignees: @lpcox @Copilot
Auth mode: PAT (COPILOT_GITHUB_TOKEN)

Overall: 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.

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

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

🔭 OTEL Smoke Test Results

Scenario Result Notes
1. Module Loading ✅ Pass otel.js loads; exports: startRequestSpan, setTokenAttributes, setBudgetAttributes, endSpan, endSpanError, shutdown, isEnabled, plus internal helpers
2. Test Suite ✅ Pass 59 tests, 2 suites (otel.test.js, otel-fanout.test.js), 0 failures
3. Env Var Forwarding ✅ Pass api-proxy-env-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, OTEL_SERVICE_NAME
4. Token Tracker Integration ✅ Pass onUsage callback exists in token-tracker-http.js (lines 283, 324, 374)
5. OTEL Diagnostics ✅ Pass No OTLP endpoint configured → graceful fallback to FileSpanExporter (/var/log/api-proxy/otel.jsonl); isEnabled() returns true

All 5 scenarios passed. OTEL tracing integration is functional.

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 commented Jul 5, 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.

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 5, 2026

Copy link
Copy Markdown
Contributor

Smoke Test Results

  • GitHub MCP Testing: ✅
  • GitHub.com Connectivity: ✅ (200)
  • File Writing: ✅
  • Bash Tool: ✅

PR Titles:

  1. [Test Coverage] branch coverage...
  2. refactor(api-proxy): centralize auth-header...

Overall status: PASS

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 5, 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 #5926 · 32.1 AIC · ⊞ 6.9K ·
Add label ready-for-aw to run again

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Pre-fetched PRs:
${{ steps.smoke-data.outputs.SMOKE_PR_DATA }}

MCP connectivity: ✅
GitHub.com connectivity: ✅
File I/O: ✅
BYOK inference: ✅

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

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.

🔑 BYOK (AOAI api-key) report filed by Smoke Copilot BYOK AOAI (api-key)
Add label ready-for-aw to run again

@lpcox lpcox merged commit 6d0d4b7 into main Jul 5, 2026
86 of 89 checks passed
@lpcox lpcox deleted the copilot/duplicate-code-handle-health-responses branch July 5, 2026 15:06
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] Provider unconfigured health responses repeat sidecar status logic

3 participants