Skip to content

Commit a43126f

Browse files
authored
Deduplicate provider adapter host/basePath/validation methods in shared adapter factory (#3210)
* Initial plan * refactor: share common provider adapter methods * docs: document participatesInValidation adapter option --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent 39720f5 commit a43126f

8 files changed

Lines changed: 61 additions & 64 deletions

File tree

containers/api-proxy/providers/ADDING-A-PROVIDER.md

Lines changed: 27 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,28 @@ Create `providers/<name>.js`. The adapter is a plain JS object (no class syntax
1717
```js
1818
'use strict';
1919

20-
const { normalizeApiTarget, normalizeBasePath } = require('../proxy-utils');
20+
const { createBaseAdapterConfig, createAdapterMethods } = require('../proxy-utils');
2121

2222
function createMyProviderAdapter(env, deps = {}) {
2323
// Read credentials and config from env at construction time
24-
const apiKey = (env.MY_PROVIDER_API_KEY || '').trim() || undefined;
25-
const target = normalizeApiTarget(env.MY_PROVIDER_API_TARGET) || 'api.myprovider.com';
26-
const basePath = normalizeBasePath(env.MY_PROVIDER_API_BASE_PATH);
24+
const { apiKey, rawTarget: target, basePath } = createBaseAdapterConfig(env, {
25+
keyEnvVar: 'MY_PROVIDER_API_KEY',
26+
targetEnvVar: 'MY_PROVIDER_API_TARGET',
27+
basePathEnvVar: 'MY_PROVIDER_API_BASE_PATH',
28+
defaultTarget: 'api.myprovider.com',
29+
});
30+
const adapterMethods = createAdapterMethods({
31+
apiKey,
32+
rawTarget: target,
33+
basePath,
34+
provider: 'my-provider',
35+
port: 10005,
36+
defaultTarget: 'api.myprovider.com',
37+
validationPath: '/v1/models',
38+
validationHeaders: () => ({ 'Authorization': `Bearer ${apiKey}` }),
39+
modelsPath: '/v1/models',
40+
modelsFetchHeaders: () => ({ 'Authorization': `Bearer ${apiKey}` }),
41+
});
2742

2843
const bodyTransform = deps.bodyTransform || null; // model-alias rewriting etc.
2944

@@ -34,12 +49,9 @@ function createMyProviderAdapter(env, deps = {}) {
3449

3550
isManagementPort: false, // true only for port 10000 (OpenAI)
3651
alwaysBind: false, // set true to start a 503-stub when not configured
37-
get participatesInValidation() { return this.isEnabled(); },
38-
3952
// ── Credentials ──────────────────────────────────────────────────────────
4053
isEnabled() { return !!apiKey; },
41-
getTargetHost() { return target; },
42-
getBasePath() { return basePath; },
54+
...adapterMethods,
4355

4456
// ── Per-request auth headers ──────────────────────────────────────────────
4557
// `req` is the incoming http.IncomingMessage — inspect it for request-specific logic.
@@ -55,41 +67,13 @@ function createMyProviderAdapter(env, deps = {}) {
5567
// Return a function (body: Buffer) => Buffer|null, or null for no transform.
5668
getBodyTransform() { return bodyTransform; },
5769

58-
// ── Startup: credential validation ───────────────────────────────────────
59-
// Return a probe config, a skip config, or null if validation is not applicable.
60-
getValidationProbe() {
61-
if (!apiKey) return null;
62-
if (target !== 'api.myprovider.com') {
63-
return { skip: true, reason: `Custom target ${target}; validation skipped` };
64-
}
65-
return {
66-
url: `https://${target}/v1/models`,
67-
opts: { method: 'GET', headers: { 'Authorization': `Bearer ${apiKey}` } },
68-
};
69-
},
70-
71-
// ── Startup: model listing ────────────────────────────────────────────────
72-
// Return null to opt out of model fetching.
73-
getModelsFetchConfig() {
74-
if (!apiKey) return null;
75-
return {
76-
url: `https://${target}/v1/models`,
77-
opts: { method: 'GET', headers: { 'Authorization': `Bearer ${apiKey}` } },
78-
cacheKey: 'my-provider', // key in cachedModels; must match name
79-
};
80-
},
81-
82-
// ── /reflect endpoint metadata ────────────────────────────────────────────
83-
getReflectionInfo() {
84-
return {
85-
provider: 'my-provider',
86-
port: 10005,
87-
base_url: 'http://api-proxy:10005',
88-
configured: !!apiKey,
89-
models_cache_key: 'my-provider', // null when models are not fetched
90-
models_url: 'http://api-proxy:10005/v1/models',
91-
};
92-
},
70+
// createAdapterMethods provides:
71+
// - participatesInValidation (defaults to !!apiKey)
72+
// - getTargetHost()
73+
// - getBasePath()
74+
// - getValidationProbe()
75+
// - getModelsFetchConfig()
76+
// - getReflectionInfo()
9377
};
9478
}
9579

containers/api-proxy/providers/anthropic.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,7 @@ function createAnthropicAdapter(env, deps = {}) {
107107
* The stub server does NOT count toward the startup validation latch —
108108
* only the fully-configured server (when ANTHROPIC_API_KEY is set) does.
109109
*/
110-
get participatesInValidation() { return this.isEnabled(); },
111-
112110
isEnabled() { return !!apiKey; },
113-
getTargetHost() { return rawTarget; },
114-
getBasePath() { return basePath; },
115111

116112
/**
117113
* Build Anthropic auth headers for this request.

containers/api-proxy/providers/copilot.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -335,11 +335,7 @@ function createCopilotAdapter(env, deps = {}) {
335335
* The stub server does NOT count toward the startup validation latch —
336336
* only the fully-configured server (when credentials are present) does.
337337
*/
338-
get participatesInValidation() { return this.isEnabled(); },
339-
340338
isEnabled() { return !!authToken; },
341-
getTargetHost() { return rawTarget; },
342-
getBasePath() { return basePath; },
343339

344340
/**
345341
* Build Copilot auth headers for this request.

containers/api-proxy/providers/gemini.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,7 @@ function createGeminiAdapter(env, deps = {}) {
5959
* The 503-fallback server does NOT count toward the startup validation latch —
6060
* only the fully-configured server (when GEMINI_API_KEY is set) does.
6161
*/
62-
get participatesInValidation() { return this.isEnabled(); },
63-
6462
isEnabled() { return !!apiKey; },
65-
getTargetHost() { return rawTarget; },
66-
getBasePath() { return basePath; },
6763

6864
getAuthHeaders() {
6965
return { 'x-goog-api-key': apiKey };

containers/api-proxy/providers/openai.js

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -126,12 +126,7 @@ function createOpenAIAdapter(env, deps = {}) {
126126
*/
127127
alwaysBind: true,
128128

129-
/** Port 10000 always counts toward the startup validation latch. */
130-
participatesInValidation: true,
131-
132129
isEnabled() { return !!apiKey || !!oidcProvider?.isReady() || !!awsOidcProvider?.isReady(); },
133-
getTargetHost() { return rawTarget; },
134-
getBasePath() { return basePath; },
135130

136131
/**
137132
* Get the OIDC token provider (Azure or GCP — Bearer-token compatible).
@@ -167,6 +162,9 @@ function createOpenAIAdapter(env, deps = {}) {
167162
getBodyTransform() { return bodyTransform; },
168163
...adapterMethods,
169164

165+
/** Port 10000 always counts toward the startup validation latch. */
166+
participatesInValidation: true,
167+
170168
/** Response returned when port 10000 receives a proxy request but no key is set. */
171169
getUnconfiguredResponse() {
172170
if (oidcConfigured) {

containers/api-proxy/providers/opencode.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,10 @@ function createOpenCodeAdapter(env, { candidateAdapters = [] } = {}) {
117117
*/
118118
alwaysBind: true,
119119

120+
// OpenCode is a routing layer over the base providers; those providers
121+
// handle their own startup validation and model fetching.
122+
...adapterMethods,
123+
120124
/**
121125
* The stub server does NOT count toward the startup validation latch —
122126
* only the fully-configured server (when enabled and a candidate is active) does.
@@ -171,10 +175,6 @@ function createOpenCodeAdapter(env, { candidateAdapters = [] } = {}) {
171175
return resolveActiveAdapter()?.getBodyTransform() || null;
172176
},
173177

174-
// OpenCode is a routing layer over the base providers; those providers
175-
// handle their own startup validation and model fetching.
176-
...adapterMethods,
177-
178178
/** Response returned for all requests when OpenCode is not configured. */
179179
getUnconfiguredResponse() {
180180
if (!enabled) {

containers/api-proxy/proxy-utils.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,13 +234,17 @@ function createBaseAdapterConfig(env, { keyEnvVar, targetEnvVar, basePathEnvVar,
234234
* @param {() => boolean} [opts.skipModelsFetch]
235235
* @param {Record<string,string>|(() => Record<string,string>)} [opts.modelsFetchHeaders]
236236
* @param {string|null} [opts.modelsCacheKey]
237+
* @param {boolean} [opts.participatesInValidation]
237238
* @param {boolean} [opts.reflectionConfigured]
238239
* @param {string|null} [opts.reflectionModelsPath]
239240
* @param {Record<string, unknown>|(() => Record<string, unknown>)} [opts.reflectionExtra]
240241
* @param {() => ({ url: string, opts: object }|{ skip: true, reason: string }|null)} [opts.getValidationProbe]
241242
* @param {() => ({ url: string, opts: object, cacheKey: string }|null)} [opts.getModelsFetchConfig]
242243
* @param {() => object} [opts.getReflectionInfo]
243244
* @returns {{
245+
* getTargetHost: (req?: import('http').IncomingMessage) => string,
246+
* getBasePath: (req?: import('http').IncomingMessage) => string,
247+
* participatesInValidation: boolean,
244248
* getValidationProbe: () => ({ url: string, opts: object }|{ skip: true, reason: string }|null),
245249
* getModelsFetchConfig: () => ({ url: string, opts: object, cacheKey: string }|null),
246250
* getReflectionInfo: () => object
@@ -263,6 +267,7 @@ function createAdapterMethods(opts) {
263267
skipModelsFetch,
264268
modelsFetchHeaders = validationHeaders,
265269
modelsCacheKey = provider,
270+
participatesInValidation = !!apiKey,
266271
reflectionConfigured = !!apiKey,
267272
reflectionModelsPath = modelsPath,
268273
reflectionExtra = {},
@@ -316,6 +321,9 @@ function createAdapterMethods(opts) {
316321
}));
317322

318323
return {
324+
getTargetHost() { return rawTarget; },
325+
getBasePath() { return basePath; },
326+
participatesInValidation,
319327
getValidationProbe: builtValidationProbe,
320328
getModelsFetchConfig: builtModelsFetchConfig,
321329
getReflectionInfo: builtReflectionInfo,

containers/api-proxy/server.routing.test.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,25 @@ describe('createAdapterMethods', () => {
8585
models_cache_key: 'example',
8686
models_url: 'http://api-proxy:12345/v1/models',
8787
});
88+
expect(methods.getTargetHost()).toBe('api.example.com');
89+
expect(methods.getBasePath()).toBe('/api/v1');
90+
});
91+
92+
it('defaults participatesInValidation from apiKey presence', () => {
93+
const methods = createAdapterMethods({
94+
rawTarget: 'api.example.com',
95+
provider: 'example',
96+
port: 12345,
97+
});
98+
const enabledMethods = createAdapterMethods({
99+
apiKey: 'sk-test',
100+
rawTarget: 'api.example.com',
101+
provider: 'example',
102+
port: 12345,
103+
});
104+
105+
expect(methods.participatesInValidation).toBe(false);
106+
expect(enabledMethods.participatesInValidation).toBe(true);
88107
});
89108

90109
it('does not double-slash model fetch URL when basePath is root', () => {

0 commit comments

Comments
 (0)