Skip to content

Commit 44ac9f3

Browse files
vraj00222claude
andcommitted
polish: address self-review feedback
• Remove unused isUrl() helper in /local parser (replaced by looksLikeUrl during earlier fix; never deleted). • Rename env var CODEBUFF_LOCAL_MODEL → CODEBUFF_PROVIDER_MODEL so all three custom-provider env vars share the CODEBUFF_(PROVIDER_)? prefix consistently. Clarify in JSDoc that the override is skipped when an agent declares its own providerOptions.baseUrl. • Default apiKey placeholder "codebuff" → "unused" in createCustomProviderModel. The literal string "codebuff" invited the wrong mental model (could read as "send my Codebuff key"); "unused" plus a comment makes the intent obvious. Local runtimes ignore the Authorization header entirely; we never send the user's real key on the direct path. • Extract maxRetries: 1 into CUSTOM_PROVIDER_MAX_RETRIES with a JSDoc explaining the choice (one retry for cold-start; more wouldn't help with deterministic local failures). • Simplify the precedence ladder in promptAiSdkStream — replace the nested ternary that paired apiKey-with-winning-baseUrl with a small sources array + .find(). Same behavior, easier to read at a glance. Tests updated for the env var rename. All 3245 tests across CLI, SDK, and agent-runtime still pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent a74e57b commit 44ac9f3

5 files changed

Lines changed: 62 additions & 56 deletions

File tree

cli/src/commands/__tests__/local-provider.test.ts

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,10 @@ describe('applyLocalAction (side effects on process.env)', () => {
153153
beforeEach(() => {
154154
originalBaseUrl = process.env.CODEBUFF_BASE_URL
155155
originalApiKey = process.env.CODEBUFF_PROVIDER_API_KEY
156-
originalModel = process.env.CODEBUFF_LOCAL_MODEL
156+
originalModel = process.env.CODEBUFF_PROVIDER_MODEL
157157
delete process.env.CODEBUFF_BASE_URL
158158
delete process.env.CODEBUFF_PROVIDER_API_KEY
159-
delete process.env.CODEBUFF_LOCAL_MODEL
159+
delete process.env.CODEBUFF_PROVIDER_MODEL
160160
})
161161

162162
afterEach(() => {
@@ -165,18 +165,18 @@ describe('applyLocalAction (side effects on process.env)', () => {
165165
if (originalApiKey === undefined)
166166
delete process.env.CODEBUFF_PROVIDER_API_KEY
167167
else process.env.CODEBUFF_PROVIDER_API_KEY = originalApiKey
168-
if (originalModel === undefined) delete process.env.CODEBUFF_LOCAL_MODEL
169-
else process.env.CODEBUFF_LOCAL_MODEL = originalModel
168+
if (originalModel === undefined) delete process.env.CODEBUFF_PROVIDER_MODEL
169+
else process.env.CODEBUFF_PROVIDER_MODEL = originalModel
170170
})
171171

172172
test('enable without model sets baseUrl, clears any previous model override', async () => {
173-
process.env.CODEBUFF_LOCAL_MODEL = 'stale-model'
173+
process.env.CODEBUFF_PROVIDER_MODEL = 'stale-model'
174174
const msg = await applyLocalAction({
175175
kind: 'enable',
176176
baseUrl: 'http://localhost:11434/v1',
177177
})
178178
expect(process.env.CODEBUFF_BASE_URL).toBe('http://localhost:11434/v1')
179-
expect(process.env.CODEBUFF_LOCAL_MODEL).toBeUndefined()
179+
expect(process.env.CODEBUFF_PROVIDER_MODEL).toBeUndefined()
180180
expect(msg).toContain('ON')
181181
expect(msg).toContain('No model override')
182182
expect(msg).toContain('llama3.1:8b')
@@ -189,7 +189,7 @@ describe('applyLocalAction (side effects on process.env)', () => {
189189
model: 'llama3.1:8b',
190190
})
191191
expect(process.env.CODEBUFF_BASE_URL).toBe('http://localhost:11434/v1')
192-
expect(process.env.CODEBUFF_LOCAL_MODEL).toBe('llama3.1:8b')
192+
expect(process.env.CODEBUFF_PROVIDER_MODEL).toBe('llama3.1:8b')
193193
expect(msg).toContain('Model override: llama3.1:8b')
194194
})
195195

@@ -198,7 +198,7 @@ describe('applyLocalAction (side effects on process.env)', () => {
198198
kind: 'set-model',
199199
model: 'llama3.1:8b',
200200
})
201-
expect(process.env.CODEBUFF_LOCAL_MODEL).toBeUndefined()
201+
expect(process.env.CODEBUFF_PROVIDER_MODEL).toBeUndefined()
202202
expect(msg).toContain('OFF')
203203
})
204204

@@ -208,16 +208,16 @@ describe('applyLocalAction (side effects on process.env)', () => {
208208
kind: 'set-model',
209209
model: 'llama3.1:8b',
210210
})
211-
expect(process.env.CODEBUFF_LOCAL_MODEL).toBe('llama3.1:8b')
211+
expect(process.env.CODEBUFF_PROVIDER_MODEL).toBe('llama3.1:8b')
212212
expect(msg).toContain('Model override: llama3.1:8b')
213213
})
214214

215215
test('clear-model removes only the model, keeps baseUrl', async () => {
216216
process.env.CODEBUFF_BASE_URL = 'http://localhost:11434/v1'
217-
process.env.CODEBUFF_LOCAL_MODEL = 'llama3.1:8b'
217+
process.env.CODEBUFF_PROVIDER_MODEL = 'llama3.1:8b'
218218
const msg = await applyLocalAction({ kind: 'clear-model' })
219219
expect(process.env.CODEBUFF_BASE_URL).toBe('http://localhost:11434/v1')
220-
expect(process.env.CODEBUFF_LOCAL_MODEL).toBeUndefined()
220+
expect(process.env.CODEBUFF_PROVIDER_MODEL).toBeUndefined()
221221
expect(msg).toContain('cleared')
222222
})
223223

@@ -229,11 +229,11 @@ describe('applyLocalAction (side effects on process.env)', () => {
229229
test('disable clears baseUrl, apiKey, and model', async () => {
230230
process.env.CODEBUFF_BASE_URL = 'http://localhost:11434/v1'
231231
process.env.CODEBUFF_PROVIDER_API_KEY = 'ollama'
232-
process.env.CODEBUFF_LOCAL_MODEL = 'llama3.1:8b'
232+
process.env.CODEBUFF_PROVIDER_MODEL = 'llama3.1:8b'
233233
const msg = await applyLocalAction({ kind: 'disable' })
234234
expect(process.env.CODEBUFF_BASE_URL).toBeUndefined()
235235
expect(process.env.CODEBUFF_PROVIDER_API_KEY).toBeUndefined()
236-
expect(process.env.CODEBUFF_LOCAL_MODEL).toBeUndefined()
236+
expect(process.env.CODEBUFF_PROVIDER_MODEL).toBeUndefined()
237237
expect(msg).toContain('OFF')
238238
expect(msg).toContain('llama3.1:8b')
239239
})
@@ -251,7 +251,7 @@ describe('applyLocalAction (side effects on process.env)', () => {
251251

252252
test('status when on with model shows both URL and model', async () => {
253253
process.env.CODEBUFF_BASE_URL = 'http://localhost:1234/v1'
254-
process.env.CODEBUFF_LOCAL_MODEL = 'llama3.1:8b'
254+
process.env.CODEBUFF_PROVIDER_MODEL = 'llama3.1:8b'
255255
const msg = await applyLocalAction({ kind: 'status' })
256256
expect(msg).toContain('ON')
257257
expect(msg).toContain('http://localhost:1234/v1')
@@ -285,48 +285,48 @@ describe('parseLocalArgs + applyLocalAction end-to-end', () => {
285285

286286
beforeEach(() => {
287287
originalBaseUrl = process.env.CODEBUFF_BASE_URL
288-
originalModel = process.env.CODEBUFF_LOCAL_MODEL
288+
originalModel = process.env.CODEBUFF_PROVIDER_MODEL
289289
delete process.env.CODEBUFF_BASE_URL
290-
delete process.env.CODEBUFF_LOCAL_MODEL
290+
delete process.env.CODEBUFF_PROVIDER_MODEL
291291
})
292292

293293
afterEach(() => {
294294
if (originalBaseUrl === undefined) delete process.env.CODEBUFF_BASE_URL
295295
else process.env.CODEBUFF_BASE_URL = originalBaseUrl
296-
if (originalModel === undefined) delete process.env.CODEBUFF_LOCAL_MODEL
297-
else process.env.CODEBUFF_LOCAL_MODEL = originalModel
296+
if (originalModel === undefined) delete process.env.CODEBUFF_PROVIDER_MODEL
297+
else process.env.CODEBUFF_PROVIDER_MODEL = originalModel
298298
})
299299

300300
test('user types `/local on llama3.1:8b` → URL default + model set', async () => {
301301
await applyLocalAction(parseLocalArgs('on llama3.1:8b'))
302302
expect(process.env.CODEBUFF_BASE_URL).toBe(DEFAULT_LOCAL_BASE_URL)
303-
expect(process.env.CODEBUFF_LOCAL_MODEL).toBe('llama3.1:8b')
303+
expect(process.env.CODEBUFF_PROVIDER_MODEL).toBe('llama3.1:8b')
304304
})
305305

306306
test('user types `/local llama3.1:8b` (no `on`) → same effect', async () => {
307307
await applyLocalAction(parseLocalArgs('llama3.1:8b'))
308308
expect(process.env.CODEBUFF_BASE_URL).toBe(DEFAULT_LOCAL_BASE_URL)
309-
expect(process.env.CODEBUFF_LOCAL_MODEL).toBe('llama3.1:8b')
309+
expect(process.env.CODEBUFF_PROVIDER_MODEL).toBe('llama3.1:8b')
310310
})
311311

312312
test('user types `/local on http://x/v1 llama3.1:8b` → both set', async () => {
313313
await applyLocalAction(parseLocalArgs('on http://x.example.com:9999/v1 llama3.1:8b'))
314314
expect(process.env.CODEBUFF_BASE_URL).toBe('http://x.example.com:9999/v1')
315-
expect(process.env.CODEBUFF_LOCAL_MODEL).toBe('llama3.1:8b')
315+
expect(process.env.CODEBUFF_PROVIDER_MODEL).toBe('llama3.1:8b')
316316
})
317317

318318
test('user types `/local model llama3.1:8b` after `/local on` → model added', async () => {
319319
await applyLocalAction(parseLocalArgs('on'))
320-
expect(process.env.CODEBUFF_LOCAL_MODEL).toBeUndefined()
320+
expect(process.env.CODEBUFF_PROVIDER_MODEL).toBeUndefined()
321321
await applyLocalAction(parseLocalArgs('model llama3.1:8b'))
322-
expect(process.env.CODEBUFF_LOCAL_MODEL).toBe('llama3.1:8b')
322+
expect(process.env.CODEBUFF_PROVIDER_MODEL).toBe('llama3.1:8b')
323323
})
324324

325325
test('user types `/local off` → both cleared', async () => {
326326
await applyLocalAction(parseLocalArgs('on llama3.1:8b'))
327327
await applyLocalAction(parseLocalArgs('off'))
328328
expect(process.env.CODEBUFF_BASE_URL).toBeUndefined()
329-
expect(process.env.CODEBUFF_LOCAL_MODEL).toBeUndefined()
329+
expect(process.env.CODEBUFF_PROVIDER_MODEL).toBeUndefined()
330330
})
331331

332332
test('mutations are visible via getter functions', async () => {

cli/src/commands/local-provider.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,6 @@ export type LocalCommandAction =
4242
| { kind: 'disable' }
4343
| { kind: 'invalid'; reason: string }
4444

45-
function isUrl(token: string): boolean {
46-
return token.startsWith('http://') || token.startsWith('https://')
47-
}
48-
4945
function looksLikeUrl(token: string): boolean {
5046
// Anything with a scheme separator — caller validates the actual scheme.
5147
return token.includes('://')

common/src/constants/custom-provider.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,9 @@ export const PROVIDER_API_KEY_ENV_VAR = 'CODEBUFF_PROVIDER_API_KEY'
99
/** Env var overriding the agent's declared model when a custom provider is active.
1010
* Used by `/local on <model>` to substitute the cloud model (e.g.
1111
* `anthropic/claude-opus-4-7`) with a model the local provider actually has
12-
* (e.g. `llama3.1:8b`). Only takes effect when PROVIDER_BASE_URL_ENV_VAR is set. */
13-
export const PROVIDER_MODEL_ENV_VAR = 'CODEBUFF_LOCAL_MODEL'
12+
* (e.g. `llama3.1:8b`).
13+
*
14+
* Only takes effect when PROVIDER_BASE_URL_ENV_VAR is set AND the agent
15+
* itself doesn't declare its own `providerOptions.baseUrl` — agents with an
16+
* explicit baseUrl are assumed to declare a matching model. */
17+
export const PROVIDER_MODEL_ENV_VAR = 'CODEBUFF_PROVIDER_MODEL'

sdk/src/impl/llm.ts

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,14 @@ type OpenRouterUsageAccounting = {
135135
}
136136
}
137137

138+
/**
139+
* Retry count for direct calls to a custom OpenAI-compatible provider.
140+
* One retry absorbs brief model-load stalls on first call. We deliberately
141+
* don't retry more — local failures are usually deterministic (provider down,
142+
* wrong URL, model not pulled) and extra retries only make errors slower.
143+
*/
144+
const CUSTOM_PROVIDER_MAX_RETRIES = 1
145+
138146
/**
139147
* Wrap raw errors from a custom OpenAI-compatible endpoint in a friendly,
140148
* actionable message. Distinguishes connection failures (provider down,
@@ -366,29 +374,25 @@ export async function* promptAiSdkStream(
366374
}
367375

368376
// Resolve custom-provider precedence: agent > client option > env.
369-
// apiKey is paired with whichever URL "wins" to avoid mixing sources.
377+
// First non-empty baseUrl wins; its apiKey comes along to avoid mixing
378+
// credentials with the wrong endpoint.
379+
const customSources = [
380+
params.agentProviderOptions,
381+
params.clientCustomProvider,
382+
{
383+
baseUrl: getCustomProviderBaseUrlFromEnv(),
384+
apiKey: getCustomProviderApiKeyFromEnv(),
385+
},
386+
]
387+
const winningSource = customSources.find((s) => s?.baseUrl)
388+
const resolvedBaseUrl = winningSource?.baseUrl
389+
const resolvedApiKey = winningSource?.apiKey
390+
391+
// Model override: substitute the agent's declared model with the env-configured
392+
// local model when the custom provider is active. Skipped when an agent
393+
// explicitly sets its own providerOptions.baseUrl — that agent is assumed to
394+
// have declared a matching model. See PROVIDER_MODEL_ENV_VAR JSDoc.
370395
const agentBaseUrl = params.agentProviderOptions?.baseUrl
371-
const agentApiKey = params.agentProviderOptions?.apiKey
372-
const clientBaseUrl = params.clientCustomProvider?.baseUrl
373-
const clientApiKey = params.clientCustomProvider?.apiKey
374-
const envBaseUrl = getCustomProviderBaseUrlFromEnv()
375-
const envApiKey = getCustomProviderApiKeyFromEnv()
376-
377-
const resolvedBaseUrl = agentBaseUrl ?? clientBaseUrl ?? envBaseUrl
378-
const resolvedApiKey = agentBaseUrl
379-
? agentApiKey
380-
: clientBaseUrl
381-
? clientApiKey
382-
: envBaseUrl
383-
? envApiKey
384-
: undefined
385-
386-
// Model override: when a custom provider is active and CODEBUFF_LOCAL_MODEL
387-
// is set, substitute the agent's declared model (which is typically a cloud
388-
// model id like 'anthropic/claude-opus-4-7' that a local provider won't
389-
// recognize) with the configured local model (e.g. 'llama3.1:8b').
390-
// Only applies to envBaseUrl/clientBaseUrl paths — an agent that explicitly
391-
// sets providerOptions.baseUrl is assumed to also have set a matching model.
392396
const envModelOverride =
393397
resolvedBaseUrl && !agentBaseUrl
394398
? getCustomProviderModelFromEnv()
@@ -438,10 +442,9 @@ export async function* promptAiSdkStream(
438442
model: aiSDKModel,
439443
messages: convertCbToModelMessages(params),
440444
// ChatGPT OAuth: no retries (we fall back to Codebuff on first failure).
441-
// Custom provider: one retry to handle brief model-load stalls without
442-
// dragging out errors when the provider is actually down.
445+
// Custom provider: see CUSTOM_PROVIDER_MAX_RETRIES.
443446
...(isChatGptOAuth ? { maxRetries: 0 } : {}),
444-
...(isCustomProvider ? { maxRetries: 1 } : {}),
447+
...(isCustomProvider ? { maxRetries: CUSTOM_PROVIDER_MAX_RETRIES } : {}),
445448
// Direct routes (ChatGPT OAuth, custom provider): skip codebuff_metadata
446449
// and OpenRouter routing keys — neither belongs in those request bodies.
447450
...(isChatGptOAuth || isCustomProvider

sdk/src/impl/model-provider.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,10 @@ function createCustomProviderModel(params: {
296296
provider: 'custom',
297297
url: ({ path: endpoint }) => `${trimmedBase}${endpoint}`,
298298
headers: () => ({
299-
Authorization: `Bearer ${apiKey ?? 'codebuff'}`,
299+
// Most local runtimes (Ollama, LM Studio) ignore the Authorization header
300+
// entirely. Send a non-empty placeholder since some servers reject empty
301+
// Bearer values; never send the user's Codebuff key on this code path.
302+
Authorization: `Bearer ${apiKey ?? 'unused'}`,
300303
'Content-Type': 'application/json',
301304
'user-agent': `ai-sdk/openai-compatible/${VERSION}/codebuff-custom-provider`,
302305
}),

0 commit comments

Comments
 (0)