Skip to content
Closed
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ import sandboxToggle from './commands/sandbox-toggle/index.js'
import chrome from './commands/chrome/index.js'
import stickers from './commands/stickers/index.js'
import advisor from './commands/advisor.js'
import provider from './commands/provider.js'
import { logError } from './utils/log.js'
import { toError } from './utils/errors.js'
import { logForDebugging } from './utils/debug.js'
Expand Down Expand Up @@ -258,6 +259,7 @@ export const INTERNAL_ONLY_COMMANDS = [
const COMMANDS = memoize((): Command[] => [
addDir,
advisor,
provider,
agents,
branch,
btw,
Expand Down
109 changes: 109 additions & 0 deletions src/commands/provider.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import type { Command } from '../commands.js'
import type { LocalCommandCall } from '../types/command.js'
import { getAPIProvider } from '../utils/model/providers.js'
import { updateSettingsForSource } from '../utils/settings/settings.js'
import { getSettings_DEPRECATED } from '../utils/settings/settings.js'
import { applyConfigEnvironmentVariables } from '../utils/managedEnv.js'
Comment on lines +1 to +6

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.

🛠️ Refactor suggestion | 🟠 Major

Use src/* alias imports in this TypeScript file.

These relative imports violate the repository import rule for TS files.

♻️ Proposed fix
-import type { Command } from '../commands.js'
-import type { LocalCommandCall } from '../types/command.js'
-import { getAPIProvider } from '../utils/model/providers.js'
-import { updateSettingsForSource } from '../utils/settings/settings.js'
-import { getSettings_DEPRECATED } from '../utils/settings/settings.js'
-import { applyConfigEnvironmentVariables } from '../utils/managedEnv.js'
+import type { Command } from 'src/commands.js'
+import type { LocalCommandCall } from 'src/types/command.js'
+import { getAPIProvider } from 'src/utils/model/providers.js'
+import { updateSettingsForSource } from 'src/utils/settings/settings.js'
+import { getSettings_DEPRECATED } from 'src/utils/settings/settings.js'
+import { applyConfigEnvironmentVariables } from 'src/utils/managedEnv.js'

As per coding guidelines, "Use src/* path alias for imports in TypeScript files."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import type { Command } from '../commands.js'
import type { LocalCommandCall } from '../types/command.js'
import { getAPIProvider } from '../utils/model/providers.js'
import { updateSettingsForSource } from '../utils/settings/settings.js'
import { getSettings_DEPRECATED } from '../utils/settings/settings.js'
import { applyConfigEnvironmentVariables } from '../utils/managedEnv.js'
import type { Command } from 'src/commands.js'
import type { LocalCommandCall } from 'src/types/command.js'
import { getAPIProvider } from 'src/utils/model/providers.js'
import { updateSettingsForSource } from 'src/utils/settings/settings.js'
import { getSettings_DEPRECATED } from 'src/utils/settings/settings.js'
import { applyConfigEnvironmentVariables } from 'src/utils/managedEnv.js'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/commands/provider.ts` around lines 1 - 6, Replace the relative imports at
the top of this file with the repository's TypeScript path alias imports
(src/*); specifically import Command and LocalCommandCall via "src/..." instead
of '../commands.js' and '../types/command.js', and replace getAPIProvider,
updateSettingsForSource, getSettings_DEPRECATED, and
applyConfigEnvironmentVariables imports to use their "src/..." module paths
(e.g., "src/utils/model/providers", "src/utils/settings/settings",
"src/utils/managedEnv") so the file uses the canonical src/* alias imports.


function getEnvVarForProvider(provider: string): string {
switch (provider) {
case 'bedrock':
return 'CLAUDE_CODE_USE_BEDROCK'
case 'vertex':
return 'CLAUDE_CODE_USE_VERTEX'
case 'foundry':
return 'CLAUDE_CODE_USE_FOUNDRY'
default:
throw new Error(`Unknown provider: ${provider}`)
}
}

// Get merged env: process.env + settings.env (from userSettings)
function getMergedEnv(): Record<string, string> {
const settings = getSettings_DEPRECATED()
const merged = { ...process.env }
if (settings?.env) {
Object.assign(merged, settings.env)
}
return merged
}
Comment on lines +22 to +29

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.

⚠️ Potential issue | 🟡 Minor

Type mismatch: process.env values can be undefined.

process.env is Record<string, string | undefined>, but getMergedEnv returns Record<string, string>. This type assertion is incorrect and could mask runtime issues.

🛡️ Suggested fix
-function getMergedEnv(): Record<string, string> {
+function getMergedEnv(): Record<string, string | undefined> {
   const settings = getSettings_DEPRECATED()
-  const merged = { ...process.env }
+  const merged: Record<string, string | undefined> = { ...process.env }
   if (settings?.env) {
     Object.assign(merged, settings.env)
   }
   return merged
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function getMergedEnv(): Record<string, string> {
const settings = getSettings_DEPRECATED()
const merged = { ...process.env }
if (settings?.env) {
Object.assign(merged, settings.env)
}
return merged
}
function getMergedEnv(): Record<string, string | undefined> {
const settings = getSettings_DEPRECATED()
const merged: Record<string, string | undefined> = { ...process.env }
if (settings?.env) {
Object.assign(merged, settings.env)
}
return merged
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/commands/provider.ts` around lines 22 - 29, getMergedEnv currently
spreads process.env (Record<string,string|undefined>) into a
Record<string,string> return which is a type mismatch; either change
getMergedEnv's signature to return Record<string,string|undefined> to match
process.env or ensure all merged values are coerced/filtered to strings before
returning (e.g., iterate merged and convert undefined to '' or String(value), or
remove keys with undefined). Update the function body and its return type
accordingly and keep references to getMergedEnv and getSettings_DEPRECATED when
making the change so callers/types remain consistent.


const call: LocalCommandCall = async (args, context) => {
const arg = args.trim().toLowerCase()

// No argument: show current provider
if (!arg) {
const current = getAPIProvider()
return { type: 'text', value: `Current API provider: ${current}` }
}

// unset - clear settings, fallback to env vars
if (arg === 'unset') {
updateSettingsForSource('userSettings', { modelType: undefined })
// Also clear all provider-specific env vars to prevent conflicts
Comment on lines +42 to +43

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.

⚠️ Potential issue | 🟠 Major

Handle updateSettingsForSource failures before returning success text.

All writes ignore { error }, so this command can report success even when persistence fails.

🛠️ Proposed fix pattern
-    updateSettingsForSource('userSettings', { modelType: undefined })
+    const unsetResult = updateSettingsForSource('userSettings', {
+      modelType: undefined,
+    })
+    if (unsetResult.error) {
+      return {
+        type: 'text',
+        value: `Failed to clear API provider: ${unsetResult.error.message}`,
+      }
+    }

Also applies to: 63-64, 83-85

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/commands/provider.ts` around lines 42 - 43, The command currently calls
updateSettingsForSource('userSettings', ...) and other persistence helpers
without checking their returned { error }, so it may report success even when
persistence fails; update the command flow (where updateSettingsForSource is
called and the subsequent provider-specific env clearing calls at the other
occurrences) to capture the result, check for error/failed status, and
return/throw or log an appropriate failure message instead of proceeding to the
success text; ensure each call (e.g., updateSettingsForSource in the provider
command handler and the similar calls at the other locations) validates the
returned value and propagates the error to the user before printing success.

delete process.env.CLAUDE_CODE_USE_BEDROCK
delete process.env.CLAUDE_CODE_USE_VERTEX
delete process.env.CLAUDE_CODE_USE_FOUNDRY
delete process.env.CLAUDE_CODE_USE_OPENAI
return { type: 'text', value: 'API provider cleared (will use environment variables).' }
}

// Validate provider
const validProviders = ['anthropic', 'openai', 'bedrock', 'vertex', 'foundry']
if (!validProviders.includes(arg)) {
return { type: 'text', value: `Invalid provider: ${arg}\nValid: ${validProviders.join(', ')}` }
}

// Check env vars when switching to openai (including settings.env)
if (arg === 'openai') {
const mergedEnv = getMergedEnv()
const hasKey = !!mergedEnv.OPENAI_API_KEY
const hasUrl = !!mergedEnv.OPENAI_BASE_URL
if (!hasKey || !hasUrl) {
updateSettingsForSource('userSettings', { modelType: 'openai' })
const missing = []
if (!hasKey) missing.push('OPENAI_API_KEY')
if (!hasUrl) missing.push('OPENAI_BASE_URL')
return {
type: 'text',
value: `Switched to OpenAI provider.\nWarning: Missing env vars: ${missing.join(', ')}\nConfigure them via /login or set manually.`,
}
}
Comment on lines +68 to +77

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.

⚠️ Potential issue | 🟠 Major

Clear cloud-provider toggles in the early OpenAI warning path.

When env vars are missing, this branch returns early and leaves prior cloud flags intact, which can keep the effective provider on bedrock/vertex/foundry.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/commands/provider.ts` around lines 62 - 71, The early-return branch that
handles missing OpenAI env vars leaves previous cloud-provider flags set, so
updateSettingsForSource('userSettings', { modelType: 'openai' }) should also
clear any cloud-specific toggles; modify the missing-env branch (the conditional
using hasKey/hasUrl) to call updateSettingsForSource with additional keys that
reset cloud providers (e.g., set bedrockEnabled/vertexEnabled/foundryEnabled or
equivalent flags to false or remove them) so the effective provider is truly
switched to OpenAI before returning the warning.

}

// Handle different provider types
// - 'anthropic' and 'openai' are stored in settings.json (persistent)
// - 'bedrock', 'vertex', 'foundry' are env-only (do NOT touch settings.json)
if (arg === 'anthropic' || arg === 'openai') {
// Clear any cloud provider env vars to avoid conflicts
delete process.env.CLAUDE_CODE_USE_BEDROCK
delete process.env.CLAUDE_CODE_USE_VERTEX
delete process.env.CLAUDE_CODE_USE_FOUNDRY
// Update settings.json
updateSettingsForSource('userSettings', { modelType: arg })
// Ensure settings.env gets applied to process.env
applyConfigEnvironmentVariables()
return { type: 'text', value: `API provider set to ${arg}.` }
Comment on lines +83 to +92

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.

⚠️ Potential issue | 🟠 Major

Provider env toggles can be overwritten by applyConfigEnvironmentVariables().

You mutate process.env and then call applyConfigEnvironmentVariables(), which reassigns env from config/settings and can reintroduce conflicting toggles. Apply config first, then enforce provider flags.

Also applies to: 92-95

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/commands/provider.ts` around lines 77 - 86, The code currently deletes
provider env toggles from process.env and then calls
applyConfigEnvironmentVariables(), which can reintroduce those toggles; to fix,
call applyConfigEnvironmentVariables() first, then enforce the provider-specific
env flags (delete process.env.CLAUDE_CODE_USE_... or set the appropriate ones)
after applying config, and keep calling updateSettingsForSource('userSettings',
{ modelType: arg }) as before; apply the same ordering change in the other
branch that handles the alternate provider (the block referenced around lines
92-95) so config is applied first and provider flags are enforced last.

} else {
// Cloud providers: set env vars only, do NOT touch settings.modelType
delete process.env.CLAUDE_CODE_USE_OPENAI
delete process.env.OPENAI_API_KEY
delete process.env.OPENAI_BASE_URL
process.env[getEnvVarForProvider(arg)] = '1'
// Do not modify settings.json - cloud providers controlled solely by env vars
applyConfigEnvironmentVariables()
return { type: 'text', value: `API provider set to ${arg} (via environment variable).` }
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

const provider = {
type: 'local',
name: 'provider',
description: 'Switch API provider (anthropic/openai/bedrock/vertex/foundry)',
aliases: ['api'],
argumentHint: '[anthropic|openai|bedrock|vertex|foundry|unset]',
supportsNonInteractive: true,
load: () => Promise.resolve({ call }),
} satisfies Command

export default provider
45 changes: 40 additions & 5 deletions src/utils/model/__tests__/providers.test.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,57 @@
import { describe, expect, test, beforeEach, afterEach } from "bun:test";
import { describe, expect, test, beforeEach, afterEach, beforeAll, afterAll } from "bun:test";
import { getAPIProvider, isFirstPartyAnthropicBaseUrl } from "../providers";
import { readFileSync, writeFileSync } from "fs";
import path from "path";
import { fileURLToPath } from "url";
import { homedir } from "os";

const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);

function getSettingsPath(): string {
return path.join(homedir(), ".claude", "settings.json");
}

describe("getAPIProvider", () => {
const envKeys = [
"CLAUDE_CODE_USE_BEDROCK",
"CLAUDE_CODE_USE_VERTEX",
"CLAUDE_CODE_USE_FOUNDRY",
"CLAUDE_CODE_USE_OPENAI",
] as const;
const savedEnv: Record<string, string | undefined> = {};
let originalSettings: string = "";

beforeAll(() => {
// Backup and clear settings.json modelType
const settingsPath = getSettingsPath();
try {
originalSettings = readFileSync(settingsPath, "utf-8");
const parsed = JSON.parse(originalSettings);
delete parsed.modelType;
writeFileSync(settingsPath, JSON.stringify(parsed, null, 2) + "\n");
} catch (e) {
// If file doesn't exist or can't parse, ignore
}
});

afterAll(() => {
// Restore original settings.json
if (originalSettings) {
writeFileSync(getSettingsPath(), originalSettings);
}
});

beforeEach(() => {
for (const key of envKeys) savedEnv[key] = process.env[key];
// Save and clear environment variables
for (const key of envKeys) {
savedEnv[key] = process.env[key];
delete process.env[key];
}
});

afterEach(() => {
// Restore environment variables
for (const key of envKeys) {
if (savedEnv[key] !== undefined) {
process.env[key] = savedEnv[key];
Expand All @@ -24,9 +62,6 @@ describe("getAPIProvider", () => {
});

test('returns "firstParty" by default', () => {
delete process.env.CLAUDE_CODE_USE_BEDROCK;
delete process.env.CLAUDE_CODE_USE_VERTEX;
delete process.env.CLAUDE_CODE_USE_FOUNDRY;
expect(getAPIProvider()).toBe("firstParty");
});

Expand Down
22 changes: 11 additions & 11 deletions src/utils/model/providers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,20 @@ import { isEnvTruthy } from '../envUtils.js'
export type APIProvider = 'firstParty' | 'bedrock' | 'vertex' | 'foundry' | 'openai'

export function getAPIProvider(): APIProvider {
// 1. Check settings.json modelType field (highest priority)
// Cloud provider env vars have highest priority (they are explicit switches)
if (isEnvTruthy(process.env.CLAUDE_CODE_USE_BEDROCK)) return 'bedrock'
if (isEnvTruthy(process.env.CLAUDE_CODE_USE_VERTEX)) return 'vertex'
if (isEnvTruthy(process.env.CLAUDE_CODE_USE_FOUNDRY)) return 'foundry'

// Check settings.json modelType field
const modelType = getInitialSettings().modelType
if (modelType === 'openai') return 'openai'

// 2. Check environment variables (backward compatibility)
return isEnvTruthy(process.env.CLAUDE_CODE_USE_OPENAI)
? 'openai'
: isEnvTruthy(process.env.CLAUDE_CODE_USE_BEDROCK)
? 'bedrock'
: isEnvTruthy(process.env.CLAUDE_CODE_USE_VERTEX)
? 'vertex'
: isEnvTruthy(process.env.CLAUDE_CODE_USE_FOUNDRY)
? 'foundry'
: 'firstParty'
// Backward compatibility: check legacy OpenAI env var
if (isEnvTruthy(process.env.CLAUDE_CODE_USE_OPENAI)) return 'openai'

// Default: Anthropic first-party API
return 'firstParty'
}

export function getAPIProviderForStatsig(): AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS {
Expand Down
Loading