From 4a6af84ad325b90481ef642966d3b535cbea6902 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 5 Jul 2026 12:50:04 +0000 Subject: [PATCH 1/3] Initial plan From b1f0e10733cd49c7b7a3b2f7e613d4ddef2131c4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 5 Jul 2026 12:56:09 +0000 Subject: [PATCH 2/3] refactor(api-proxy): deduplicate getUnconfiguredHealthResponse via declarative 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 --- containers/api-proxy/adapter-factory.js | 24 ++++++- containers/api-proxy/adapter-factory.test.js | 67 +++++++++++++++++++ .../api-proxy/providers/ADDING-A-PROVIDER.md | 7 +- containers/api-proxy/providers/anthropic.js | 11 +-- containers/api-proxy/providers/copilot.js | 11 +-- containers/api-proxy/providers/gemini.js | 8 +-- containers/api-proxy/providers/vertex.js | 7 +- 7 files changed, 106 insertions(+), 29 deletions(-) diff --git a/containers/api-proxy/adapter-factory.js b/containers/api-proxy/adapter-factory.js index 34fcc8f9d..9d7d15663 100644 --- a/containers/api-proxy/adapter-factory.js +++ b/containers/api-proxy/adapter-factory.js @@ -12,7 +12,7 @@ 'use strict'; -const { normalizeApiTarget, normalizeBasePath } = require('./proxy-utils'); +const { normalizeApiTarget, normalizeBasePath, makeUnconfiguredHealthResponse } = require('./proxy-utils'); /** * @@ -204,7 +204,10 @@ function createProviderAuthScaffold(env, deps = {}, { keyEnvVar, targetEnvVar, b * @param {(() => boolean)} [opts.isEnabled] - Optional isEnabled override (must be provided either here or via `extra`) * @param {((url: string) => string)} [opts.transformRequestUrl] - Optional URL transformer * @param {(() => import('./providers/index').UnconfiguredResponse)} [opts.getUnconfiguredResponse] - Optional not-configured response - * @param {(() => import('./providers/index').UnconfiguredResponse)} [opts.getUnconfiguredHealthResponse] - Optional not-configured /health response + * @param {(() => import('./providers/index').UnconfiguredResponse)} [opts.getUnconfiguredHealthResponse] - Optional explicit not-configured /health response (takes precedence over declarative metadata) + * @param {string} [opts.healthServiceName] - Service name for auto-generated /health response (e.g. 'awf-api-proxy-gemini'); requires missingCredentialMessage + * @param {string} [opts.missingCredentialMessage] - Default error message when credentials are absent; requires healthServiceName + * @param {(() => { message: string, status?: string }|null)} [opts.unavailableWhen] - Optional callback; when it returns a non-null object the /health response uses that message (and optional status, e.g. 'unavailable') instead of missingCredentialMessage * @param {object} [opts.extra={}] - Extra fields spread into the returned object last (e.g. OIDC runtime methods, introspection fields, overrides) * @returns {import('./providers/index').ProviderAdapter} */ @@ -220,8 +223,25 @@ function buildProviderAdapter({ transformRequestUrl, getUnconfiguredResponse, getUnconfiguredHealthResponse, + healthServiceName, + missingCredentialMessage, + unavailableWhen, extra = {}, }) { + // 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); + }; + } const adapter = { name, port, diff --git a/containers/api-proxy/adapter-factory.test.js b/containers/api-proxy/adapter-factory.test.js index 10e54941e..25c1aef64 100644 --- a/containers/api-proxy/adapter-factory.test.js +++ b/containers/api-proxy/adapter-factory.test.js @@ -208,6 +208,73 @@ describe('buildProviderAdapter', () => { }); expect(adapter.getUnconfiguredHealthResponse).toBe(getUnconfiguredHealthResponse); }); + + it('auto-generates getUnconfiguredHealthResponse from healthServiceName and missingCredentialMessage', () => { + const adapter = buildProviderAdapter({ + name: 'test', + port: 10099, + adapterMethods: makeAdapterMethods(), + getAuthHeaders() { return {}; }, + isEnabled() { return false; }, + healthServiceName: 'awf-api-proxy-test', + missingCredentialMessage: 'TEST_API_KEY not configured in api-proxy sidecar', + }); + expect(typeof adapter.getUnconfiguredHealthResponse).toBe('function'); + const response = adapter.getUnconfiguredHealthResponse(); + expect(response.statusCode).toBe(503); + expect(response.body.status).toBe('not_configured'); + expect(response.body.service).toBe('awf-api-proxy-test'); + expect(response.body.error).toBe('TEST_API_KEY not configured in api-proxy sidecar'); + }); + + it('auto-generated getUnconfiguredHealthResponse uses unavailableWhen override when it returns truthy', () => { + const adapter = buildProviderAdapter({ + name: 'test', + port: 10099, + adapterMethods: makeAdapterMethods(), + getAuthHeaders() { return {}; }, + isEnabled() { return false; }, + healthServiceName: 'awf-api-proxy-test', + missingCredentialMessage: 'TEST_API_KEY not configured in api-proxy sidecar', + unavailableWhen: () => ({ message: 'OIDC token unavailable', status: 'unavailable' }), + }); + const response = adapter.getUnconfiguredHealthResponse(); + expect(response.statusCode).toBe(503); + expect(response.body.status).toBe('unavailable'); + expect(response.body.service).toBe('awf-api-proxy-test'); + expect(response.body.error).toBe('OIDC token unavailable'); + }); + + it('auto-generated getUnconfiguredHealthResponse falls back to missingCredentialMessage when unavailableWhen returns null', () => { + const adapter = buildProviderAdapter({ + name: 'test', + port: 10099, + adapterMethods: makeAdapterMethods(), + getAuthHeaders() { return {}; }, + isEnabled() { return false; }, + healthServiceName: 'awf-api-proxy-test', + missingCredentialMessage: 'TEST_API_KEY not configured in api-proxy sidecar', + unavailableWhen: () => null, + }); + const response = adapter.getUnconfiguredHealthResponse(); + expect(response.body.status).toBe('not_configured'); + expect(response.body.error).toBe('TEST_API_KEY not configured in api-proxy sidecar'); + }); + + it('explicit getUnconfiguredHealthResponse takes precedence over declarative metadata', () => { + const explicitFn = () => ({ statusCode: 503, body: { status: 'custom' } }); + const adapter = buildProviderAdapter({ + name: 'test', + port: 10099, + adapterMethods: makeAdapterMethods(), + getAuthHeaders() { return {}; }, + isEnabled() { return false; }, + getUnconfiguredHealthResponse: explicitFn, + healthServiceName: 'awf-api-proxy-test', + missingCredentialMessage: 'TEST_API_KEY not configured', + }); + expect(adapter.getUnconfiguredHealthResponse).toBe(explicitFn); + }); }); describe('extra fields', () => { diff --git a/containers/api-proxy/providers/ADDING-A-PROVIDER.md b/containers/api-proxy/providers/ADDING-A-PROVIDER.md index debf97479..411cf58c2 100644 --- a/containers/api-proxy/providers/ADDING-A-PROVIDER.md +++ b/containers/api-proxy/providers/ADDING-A-PROVIDER.md @@ -102,7 +102,12 @@ module.exports = { createMyProviderAdapter }; | `getModelsFetchConfig()` | ✅ | Returns fetch config or `null` | | `getReflectionInfo()` | ✅ | Returns endpoint metadata for `/reflect` and `models.json` | | `getUnconfiguredResponse()` | ➖ optional | Response for proxy requests when `alwaysBind=true` & not enabled | -| `getUnconfiguredHealthResponse()` | ➖ optional | `/health` response when not enabled (defaults to 503) | +| `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) | + +> **Tip:** Pass `healthServiceName` + `missingCredentialMessage` (and optionally `unavailableWhen`) to `buildProviderAdapter` instead of writing a `getUnconfiguredHealthResponse()` method. The factory auto-generates the method from those values, keeping provider files free of repetitive boilerplate. --- diff --git a/containers/api-proxy/providers/anthropic.js b/containers/api-proxy/providers/anthropic.js index 67aa5cdde..6edcdffff 100644 --- a/containers/api-proxy/providers/anthropic.js +++ b/containers/api-proxy/providers/anthropic.js @@ -15,7 +15,6 @@ const { composeBodyTransforms, makeProviderNotConfiguredResponse, - makeUnconfiguredHealthResponse, } = require('../proxy-utils'); const { validateAuthHeaderEnv, @@ -222,13 +221,9 @@ function createAnthropicAdapter(env, deps = {}) { 'Credentials for Anthropic (port 10001) are not configured. Set ANTHROPIC_API_KEY to enable this provider.' ); }, - /** /health response when not configured. */ - getUnconfiguredHealthResponse() { - if (oidcRequested) { - return makeUnconfiguredHealthResponse('awf-api-proxy-anthropic', oidcUnavailableError, 'unavailable'); - } - return makeUnconfiguredHealthResponse('awf-api-proxy-anthropic', 'ANTHROPIC_API_KEY not configured in api-proxy sidecar'); - }, + healthServiceName: 'awf-api-proxy-anthropic', + missingCredentialMessage: 'ANTHROPIC_API_KEY not configured in api-proxy sidecar', + unavailableWhen: () => oidcRequested ? { message: oidcUnavailableError, status: 'unavailable' } : null, extra: { ...oidcRuntimeMethods, // Exposed for introspection (logging, tests) diff --git a/containers/api-proxy/providers/copilot.js b/containers/api-proxy/providers/copilot.js index 18c1a8a96..8b74e7cb9 100644 --- a/containers/api-proxy/providers/copilot.js +++ b/containers/api-proxy/providers/copilot.js @@ -20,7 +20,6 @@ const { normalizeBasePath, makeProviderNotConfiguredResponse, - makeUnconfiguredHealthResponse, composeBodyTransforms, } = require('../proxy-utils'); const { resolveOidcAuthHeaders } = require('../oidc-adapter-utils'); @@ -262,13 +261,9 @@ function buildCopilotModelsRequest(extra = {}) { 'Credentials for GitHub Copilot (port 10002) are not configured. Set COPILOT_GITHUB_TOKEN or COPILOT_PROVIDER_API_KEY to enable this provider.' ); }, - /** /health response when not configured. */ - 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'); - }, + 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, extra: { ...oidcRuntimeMethods, // Exposed for introspection / testing diff --git a/containers/api-proxy/providers/gemini.js b/containers/api-proxy/providers/gemini.js index 9a0733115..74b844e2e 100644 --- a/containers/api-proxy/providers/gemini.js +++ b/containers/api-proxy/providers/gemini.js @@ -13,7 +13,7 @@ * Gemini SDK versions append alongside the header. */ -const { stripGeminiKeyParam, makeUnconfiguredHealthResponse } = require('../proxy-utils'); +const { stripGeminiKeyParam } = require('../proxy-utils'); const { createProviderAuthScaffold, createAdapterMethods, buildProviderAdapter } = require('../adapter-factory'); const { GEMINI_ENV } = require('../provider-env-constants'); const { providerKeyHeaders } = require('./auth-headers'); @@ -75,10 +75,8 @@ function createGeminiAdapter(env, deps = {}) { body: { error: 'Gemini proxy not configured (no GEMINI_API_KEY). Set GEMINI_API_KEY in the AWF runner environment to enable credential isolation.' }, }; }, - /** /health response when not configured. */ - getUnconfiguredHealthResponse() { - return makeUnconfiguredHealthResponse('awf-api-proxy-gemini', 'GEMINI_API_KEY not configured in api-proxy sidecar'); - }, + healthServiceName: 'awf-api-proxy-gemini', + missingCredentialMessage: 'GEMINI_API_KEY not configured in api-proxy sidecar', }); } diff --git a/containers/api-proxy/providers/vertex.js b/containers/api-proxy/providers/vertex.js index dd1dac6bc..3d9c79dc5 100644 --- a/containers/api-proxy/providers/vertex.js +++ b/containers/api-proxy/providers/vertex.js @@ -15,7 +15,6 @@ * aiplatform.googleapis.com directly, enabling credential isolation. */ -const { makeUnconfiguredHealthResponse } = require('../proxy-utils'); const { createProviderAuthScaffold, createAdapterMethods, buildProviderAdapter } = require('../adapter-factory'); const { VERTEX_ENV } = require('../provider-env-constants'); const { providerKeyHeaders } = require('./auth-headers'); @@ -67,10 +66,8 @@ function createVertexAdapter(env, deps = {}) { body: { error: 'Vertex AI proxy not configured (no GOOGLE_API_KEY). Set GOOGLE_API_KEY in the AWF runner environment to enable credential isolation.' }, }; }, - /** /health response when not configured. */ - getUnconfiguredHealthResponse() { - return makeUnconfiguredHealthResponse('awf-api-proxy-vertex', 'GOOGLE_API_KEY not configured in api-proxy sidecar'); - }, + healthServiceName: 'awf-api-proxy-vertex', + missingCredentialMessage: 'GOOGLE_API_KEY not configured in api-proxy sidecar', }); } From 12e2ea658cca6e37e372b30ee817a2b70032d6a0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 5 Jul 2026 13:24:19 +0000 Subject: [PATCH 3/3] fix(api-proxy): validate declarative health metadata contract --- containers/api-proxy/adapter-factory.js | 9 +++++- containers/api-proxy/adapter-factory.test.js | 29 +++++++++++++++++++ .../api-proxy/providers/ADDING-A-PROVIDER.md | 2 +- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/containers/api-proxy/adapter-factory.js b/containers/api-proxy/adapter-factory.js index 9d7d15663..3d53db835 100644 --- a/containers/api-proxy/adapter-factory.js +++ b/containers/api-proxy/adapter-factory.js @@ -231,7 +231,14 @@ function buildProviderAdapter({ // 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) { + const hasDeclarativeHealthMetadata = + healthServiceName !== undefined || missingCredentialMessage !== undefined || unavailableWhen !== undefined; + if (getUnconfiguredHealthResponse === undefined && hasDeclarativeHealthMetadata) { + if (healthServiceName === undefined || missingCredentialMessage === undefined) { + throw new TypeError( + `Provider adapter "${name}" declarative health metadata requires both healthServiceName and missingCredentialMessage`, + ); + } getUnconfiguredHealthResponse = () => { if (unavailableWhen) { const override = unavailableWhen(); diff --git a/containers/api-proxy/adapter-factory.test.js b/containers/api-proxy/adapter-factory.test.js index 25c1aef64..566d09da4 100644 --- a/containers/api-proxy/adapter-factory.test.js +++ b/containers/api-proxy/adapter-factory.test.js @@ -227,6 +227,35 @@ describe('buildProviderAdapter', () => { expect(response.body.error).toBe('TEST_API_KEY not configured in api-proxy sidecar'); }); + it('throws when declarative health metadata is partially specified', () => { + expect(() => buildProviderAdapter({ + name: 'test', + port: 10099, + adapterMethods: makeAdapterMethods(), + getAuthHeaders() { return {}; }, + isEnabled() { return false; }, + healthServiceName: 'awf-api-proxy-test', + })).toThrow('declarative health metadata requires both healthServiceName and missingCredentialMessage'); + + expect(() => buildProviderAdapter({ + name: 'test', + port: 10099, + adapterMethods: makeAdapterMethods(), + getAuthHeaders() { return {}; }, + isEnabled() { return false; }, + missingCredentialMessage: 'TEST_API_KEY not configured in api-proxy sidecar', + })).toThrow('declarative health metadata requires both healthServiceName and missingCredentialMessage'); + + expect(() => buildProviderAdapter({ + name: 'test', + port: 10099, + adapterMethods: makeAdapterMethods(), + getAuthHeaders() { return {}; }, + isEnabled() { return false; }, + unavailableWhen: () => ({ message: 'OIDC token unavailable' }), + })).toThrow('declarative health metadata requires both healthServiceName and missingCredentialMessage'); + }); + it('auto-generated getUnconfiguredHealthResponse uses unavailableWhen override when it returns truthy', () => { const adapter = buildProviderAdapter({ name: 'test', diff --git a/containers/api-proxy/providers/ADDING-A-PROVIDER.md b/containers/api-proxy/providers/ADDING-A-PROVIDER.md index 411cf58c2..6d4604206 100644 --- a/containers/api-proxy/providers/ADDING-A-PROVIDER.md +++ b/containers/api-proxy/providers/ADDING-A-PROVIDER.md @@ -105,7 +105,7 @@ module.exports = { createMyProviderAdapter }; | `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) | +| `unavailableWhen` | ➖ optional | `() => ({ message: 'OIDC token unavailable', status: 'unavailable' })` or `() => null` — when non-null the auto-generated `/health` response uses the returned message/status (e.g. for OIDC token-not-yet-available states) | > **Tip:** Pass `healthServiceName` + `missingCredentialMessage` (and optionally `unavailableWhen`) to `buildProviderAdapter` instead of writing a `getUnconfiguredHealthResponse()` method. The factory auto-generates the method from those values, keeping provider files free of repetitive boilerplate.