provider (api) 指令查询/切换模型api类型 分离OpenAl和Anthropic模型的环境变量#129
Open
2228293026 wants to merge 23 commits intoclaude-code-best:mainfrom
Open
provider (api) 指令查询/切换模型api类型 分离OpenAl和Anthropic模型的环境变量#1292228293026 wants to merge 23 commits intoclaude-code-best:mainfrom
2228293026 wants to merge 23 commits intoclaude-code-best:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds OpenAI-family default model environment variables and reorders model-resolution precedence to prefer those OpenAI vars; introduces a new local Changes
Sequence DiagramsequenceDiagram
participant User as User (CLI/UI)
participant Cmd as Provider Command
participant Settings as Settings Manager
participant Env as Env Vars
participant Detector as Provider Detector
User->>Cmd: run `provider openai` (or other)
Cmd->>Cmd: validate argument against allowlist
Cmd->>Env: read merged env (OPENAI_API_KEY, OPENAI_BASE_URL, provider flags)
Env-->>Cmd: present / missing
alt missing keys
Cmd->>Settings: updateSettingsForSource('userSettings', { modelType: 'openai' })
Settings-->>Cmd: persisted (warning)
Cmd-->>User: warn missing env vars
else keys present
Cmd->>Env: clear cloud-provider flags
Cmd->>Settings: updateSettingsForSource('userSettings', { modelType: 'openai' })
Cmd->>Env: applyConfigEnvironmentVariables()
Cmd-->>User: success
end
User->>Detector: getAPIProvider()
Detector->>Env: check CLAUDE_CODE_USE_* flags, CLAUDE_CODE_USE_OPENAI
Detector->>Settings: read initial modelType
Detector-->>User: returns provider (e.g., 'openai' or 'firstParty')
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/model/modelSupportOverrides.ts (1)
45-67:⚠️ Potential issue | 🟠 MajorMemoization cache key doesn't include provider, causing stale results after provider switch.
The memoized function calls
getAPIProvider()at line 47 and 52 to determine which tier list to use, but the cache key at line 66 only includesmodelandcapability. After a user switches providers via/provider, the cached result for a given model+capability pair will reflect the old provider's tier list, not the new one.🐛 Proposed fix to include provider in cache key
export const get3PModelCapabilityOverride = memoize( (model: string, capability: ModelCapabilityOverride): boolean | undefined => { - if (getAPIProvider() === 'firstParty') { + const provider = getAPIProvider() + if (provider === 'firstParty') { return undefined } const m = model.toLowerCase() // Choose the appropriate tier list based on provider - const tiers = getAPIProvider() === 'openai' ? OPENAI_TIERS : ANTHROPIC_TIERS + const tiers = provider === 'openai' ? OPENAI_TIERS : ANTHROPIC_TIERS for (const tier of tiers) { const pinned = process.env[tier.modelEnvVar] const capabilities = process.env[tier.capabilitiesEnvVar] if (!pinned || capabilities === undefined) continue if (m !== pinned.toLowerCase()) continue return capabilities .toLowerCase() .split(',') .map(s => s.trim()) .includes(capability) } return undefined }, - (model, capability) => `${model.toLowerCase()}:${capability}`, + (model, capability) => `${getAPIProvider()}:${model.toLowerCase()}:${capability}`, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/model/modelSupportOverrides.ts` around lines 45 - 67, The memoized function get3PModelCapabilityOverride uses getAPIProvider() internally but the memoize cache key currently only includes model and capability, causing stale results when the provider changes; update the memoize resolver to include the current provider (call getAPIProvider() and incorporate its value into the cache key string) so the key uniquely identifies provider+model+capability and ensures cache entries are invalid when the provider switches.
🧹 Nitpick comments (6)
src/utils/model/modelOptions.ts (2)
119-145: Same redundant pattern in Opus and Haiku functions.The same
getAPIProvider()redundancy exists ingetCustomOpusOption()andgetCustomHaikuOption(). Consider applying the same optimization if you refactor.Also applies to: 188-213
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/model/modelOptions.ts` around lines 119 - 145, getCustomOpusOption (and similarly getCustomHaikuOption) repeatedly calls getAPIProvider(), which is redundant; capture the provider once (e.g., const provider = getAPIProvider()) and compute is3P from that (const is3P = provider !== 'firstParty'), then use provider === 'openai' for selecting customOpusModel, nameEnv, and descEnv (and analogously for Haiku) so all provider-dependent decisions reference the single provider variable instead of calling getAPIProvider() multiple times.
77-103: Logic is correct, butgetAPIProvider()is called redundantly.The OpenAI-specific env var resolution works correctly. However,
getAPIProvider()is called 3 times within this function (lines 81, 89, 93), and the result is constant. Consider extracting it to a local variable for clarity.♻️ Optional: Extract provider check to reduce redundancy
function getCustomSonnetOption(): ModelOption | undefined { const is3P = getAPIProvider() !== 'firstParty' + const isOpenAI = getAPIProvider() === 'openai' // For OpenAI provider, use OPENAI_DEFAULT_SONNET_MODEL; for other 3P, use ANTHROPIC_DEFAULT_SONNET_MODEL const customSonnetModel = - getAPIProvider() === 'openai' + isOpenAI ? process.env.OPENAI_DEFAULT_SONNET_MODEL : process.env.ANTHROPIC_DEFAULT_SONNET_MODEL // When a 3P user has a custom sonnet model string, show it directly if (is3P && customSonnetModel) { const is1m = has1mContext(customSonnetModel) // Use appropriate NAME/DESCRIPTION env vars based on provider const nameEnv = - getAPIProvider() === 'openai' + isOpenAI ? process.env.OPENAI_DEFAULT_SONNET_MODEL_NAME : process.env.ANTHROPIC_DEFAULT_SONNET_MODEL_NAME const descEnv = - getAPIProvider() === 'openai' + isOpenAI ? process.env.OPENAI_DEFAULT_SONNET_MODEL_DESCRIPTION : process.env.ANTHROPIC_DEFAULT_SONNET_MODEL_DESCRIPTION🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/model/modelOptions.ts` around lines 77 - 103, In getCustomSonnetOption(), reduce redundant calls to getAPIProvider() by calling it once into a local const (e.g., const provider = getAPIProvider()) and then use provider === 'openai' and provider !== 'firstParty' instead of repeated getAPIProvider() calls; update the existing conditional logic that sets is3P, customSonnetModel, nameEnv, and descEnv to reference this local provider variable so the function is clearer and avoids repeated evaluations.src/services/api/openai/__tests__/modelMapping.test.ts (1)
7-9: Test isolation is correct, but missing coverage for new priority behavior.The env var preservation/cleanup is properly extended. However, there are no test cases verifying that
OPENAI_DEFAULT_{FAMILY}_MODELtakes precedence overANTHROPIC_DEFAULT_{FAMILY}_MODEL, which is a key behavioral change inresolveOpenAIModel.🧪 Suggested test to verify OpenAI env var priority
+ test('OPENAI_DEFAULT_SONNET_MODEL takes precedence over ANTHROPIC_DEFAULT_SONNET_MODEL', () => { + process.env.OPENAI_DEFAULT_SONNET_MODEL = 'openai-sonnet' + process.env.ANTHROPIC_DEFAULT_SONNET_MODEL = 'anthropic-sonnet' + expect(resolveOpenAIModel('claude-sonnet-4-6')).toBe('openai-sonnet') + }) + + test('falls back to ANTHROPIC_DEFAULT_SONNET_MODEL when OPENAI_DEFAULT_SONNET_MODEL not set', () => { + process.env.ANTHROPIC_DEFAULT_SONNET_MODEL = 'anthropic-sonnet' + expect(resolveOpenAIModel('claude-sonnet-4-6')).toBe('anthropic-sonnet') + })Also applies to: 17-19
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/api/openai/__tests__/modelMapping.test.ts` around lines 7 - 9, Add unit tests in modelMapping.test.ts that assert resolveOpenAIModel prefers OPENAI_DEFAULT_{FAMILY}_MODEL over ANTHROPIC_DEFAULT_{FAMILY}_MODEL for each family (HAIKU, SONNET, OPUS): set both env vars (e.g., OPENAI_DEFAULT_HAIKU_MODEL and ANTHROPIC_DEFAULT_HAIKU_MODEL) to distinct values, call resolveOpenAIModel("HAIKU") and expect the OPENAI value to be returned; repeat for "SONNET" and "OPUS". Preserve and restore existing env vars as the other tests do so cleanup remains intact.src/utils/model/__tests__/providers.test.ts (2)
8-9: Unused variables__filenameand__dirname.These variables are defined but never used in the test file. Consider removing them along with the
pathandfileURLToPathimports if they're not needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/model/__tests__/providers.test.ts` around lines 8 - 9, Remove the unused __filename and __dirname variables and their creation code in providers.test.ts (the const __filename = fileURLToPath(import.meta.url); and const __dirname = path.dirname(__filename); lines), and also remove the corresponding unused imports for path and fileURLToPath at the top of the file to eliminate dead code and linter warnings.
64-66: Missing test coverage forCLAUDE_CODE_USE_OPENAI.The
envKeysarray includesCLAUDE_CODE_USE_OPENAI(line 20), but there's no corresponding test case verifying thatgetAPIProvider()returns"openai"when this env var is set. The other provider env vars (bedrock, vertex, foundry) all have dedicated test cases.➕ Suggested test to add
test('returns "openai" when CLAUDE_CODE_USE_OPENAI is set', () => { process.env.CLAUDE_CODE_USE_OPENAI = "1"; expect(getAPIProvider()).toBe("openai"); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/model/__tests__/providers.test.ts` around lines 64 - 66, Add a test for the CLAUDE_CODE_USE_OPENAI env var: set process.env.CLAUDE_CODE_USE_OPENAI = "1", call getAPIProvider(), assert it returns "openai", and then restore/cleanup the env var to avoid test leakage; place this alongside the existing provider tests in providers.test.ts and reference the getAPIProvider function and the CLAUDE_CODE_USE_OPENAI env var.src/commands/provider.ts (1)
1-6: Imports use relative paths instead ofsrc/*path alias.As per coding guidelines, TypeScript files should use
src/*path alias for imports.♻️ Suggested 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, 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".🤖 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, The imports at the top of provider.ts use relative paths; update them to use the TypeScript path alias (src/*) instead. Replace imports for Command, LocalCommandCall, getAPIProvider, updateSettingsForSource, getSettings_DEPRECATED, and applyConfigEnvironmentVariables so they import from their corresponding src/... modules (e.g. src/commands, src/types/command, src/utils/model/providers, src/utils/settings/settings, src/utils/managedEnv) instead of '../' relative paths; keep the imported symbols and exact names unchanged so resolve/tsconfig path mapping is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/provider.ts`:
- Around line 88-95: The deletion of provider-related env vars from process.env
(the calls around delete process.env.CLAUDE_CODE_USE_OPENAI, OPENAI_API_KEY,
OPENAI_BASE_URL and the earlier deletions) is undone when
applyConfigEnvironmentVariables() merges settings.env via
filterSettingsEnv(getSettings_DEPRECATED()?.env); to fix, remove these keys from
the active settings env before re-applying or persist the change via the
settings updater: call updateSettingsForSource (or otherwise remove the keys
from getSettings_DEPRECATED()?.env) for the same keys returned by
getEnvVarForProvider(arg) and the explicit
CLAUDE_CODE_USE_OPENAI/OPENAI_API_KEY/OPENAI_BASE_URL entries so that
applyConfigEnvironmentVariables() won’t re-add them, then call
applyConfigEnvironmentVariables() as before; reference
applyConfigEnvironmentVariables, getEnvVarForProvider, getSettings_DEPRECATED,
filterSettingsEnv, and updateSettingsForSource to locate and implement the
change.
- Around line 57-72: When switching provider to 'openai' (the block checking if
arg === 'openai' using getMergedEnv and hasKey/hasUrl), ensure
applyConfigEnvironmentVariables() is invoked before returning when env vars are
missing: after calling updateSettingsForSource('userSettings', { modelType:
'openai' }) and before building/returning the warning response, call
applyConfigEnvironmentVariables() so any existing OpenAI settings are applied to
process.env; keep the early-return behavior but add that call to mirror the
later anthropic/openai flow.
In `@src/components/ConsoleOAuthFlow.tsx`:
- Around line 614-616: The code calls onDone() immediately after
setOAuthStatus({ state: 'success' }) in the custom_platform and openai_chat_api
branches, which can race with rendering/unmount; change those sites (where
setOAuthStatus and onDone are used in the custom_platform branch and in the
openai_chat_api branch) to defer calling onDone until after the success state
has a chance to render — e.g., mirror the OAuth success path by wrapping the
onDone() invocation in a short setTimeout (or requestAnimationFrame) so state
updates and cleanup effects run before navigation/unmount.
- Around line 631-654: The two useKeybinding hook calls inside ConsoleOAuthFlow
(the handlers that reference FIELDS, activeField, setOAuthStatus, buildState,
displayValues, setInputValue, and setInputCursorOffset) violate the Rules of
Hooks because they run conditionally; extract them into a separate child
component (e.g., OAuthFieldKeybindings) that always mounts and receives props
(activeField, inputValue, displayValues, setOAuthStatus, setInputValue,
setInputCursorOffset) and move both 'tabs:next' and 'tabs:previous'
useKeybinding calls into that component so hooks are unconditionally declared
and logic remains identical.
- Around line 547-548: The useState hooks (inputValue/setInputValue and
inputCursorOffset/setInputCursorOffset) are being called conditionally inside
the OAuthStatusMessage switch cases (notably the custom_platform and
openai_chat_api branches), which violates the Rules of Hooks; move these
useState declarations to the top-level of the OAuthStatusMessage component (so
they run unconditionally) or extract the custom_platform and openai_chat_api
branches into their own components (e.g., CustomPlatformField and
OpenAIChatField) and call the hooks there, ensuring you keep references to
displayValues and activeField to initialize/update inputValue and
inputCursorOffset appropriately.
In `@src/utils/model/__tests__/providers.test.ts`:
- Around line 25-43: The tests currently mutate the real user settings by
calling getSettingsPath() inside beforeAll/afterAll and only restoring when
originalSettings is truthy; change the setup to use an isolated test settings
file or mock file-access: replace calls to getSettingsPath() in the test scope
with a test-specific path (e.g., create a temp dir/file via os.tmpdir() or
fs.mkdtemp) or mock getSettingsPath/readFileSync/writeFileSync so all I/O
targets that temp file; also ensure originalSettings is tracked distinctly (use
undefined vs empty string) and always restore in afterAll if originalSettings
!== undefined (or use try/finally) so an empty settings file is restored even if
tests crash or throw.
---
Outside diff comments:
In `@src/utils/model/modelSupportOverrides.ts`:
- Around line 45-67: The memoized function get3PModelCapabilityOverride uses
getAPIProvider() internally but the memoize cache key currently only includes
model and capability, causing stale results when the provider changes; update
the memoize resolver to include the current provider (call getAPIProvider() and
incorporate its value into the cache key string) so the key uniquely identifies
provider+model+capability and ensures cache entries are invalid when the
provider switches.
---
Nitpick comments:
In `@src/commands/provider.ts`:
- Around line 1-6: The imports at the top of provider.ts use relative paths;
update them to use the TypeScript path alias (src/*) instead. Replace imports
for Command, LocalCommandCall, getAPIProvider, updateSettingsForSource,
getSettings_DEPRECATED, and applyConfigEnvironmentVariables so they import from
their corresponding src/... modules (e.g. src/commands, src/types/command,
src/utils/model/providers, src/utils/settings/settings, src/utils/managedEnv)
instead of '../' relative paths; keep the imported symbols and exact names
unchanged so resolve/tsconfig path mapping is used.
In `@src/services/api/openai/__tests__/modelMapping.test.ts`:
- Around line 7-9: Add unit tests in modelMapping.test.ts that assert
resolveOpenAIModel prefers OPENAI_DEFAULT_{FAMILY}_MODEL over
ANTHROPIC_DEFAULT_{FAMILY}_MODEL for each family (HAIKU, SONNET, OPUS): set both
env vars (e.g., OPENAI_DEFAULT_HAIKU_MODEL and ANTHROPIC_DEFAULT_HAIKU_MODEL) to
distinct values, call resolveOpenAIModel("HAIKU") and expect the OPENAI value to
be returned; repeat for "SONNET" and "OPUS". Preserve and restore existing env
vars as the other tests do so cleanup remains intact.
In `@src/utils/model/__tests__/providers.test.ts`:
- Around line 8-9: Remove the unused __filename and __dirname variables and
their creation code in providers.test.ts (the const __filename =
fileURLToPath(import.meta.url); and const __dirname = path.dirname(__filename);
lines), and also remove the corresponding unused imports for path and
fileURLToPath at the top of the file to eliminate dead code and linter warnings.
- Around line 64-66: Add a test for the CLAUDE_CODE_USE_OPENAI env var: set
process.env.CLAUDE_CODE_USE_OPENAI = "1", call getAPIProvider(), assert it
returns "openai", and then restore/cleanup the env var to avoid test leakage;
place this alongside the existing provider tests in providers.test.ts and
reference the getAPIProvider function and the CLAUDE_CODE_USE_OPENAI env var.
In `@src/utils/model/modelOptions.ts`:
- Around line 119-145: getCustomOpusOption (and similarly getCustomHaikuOption)
repeatedly calls getAPIProvider(), which is redundant; capture the provider once
(e.g., const provider = getAPIProvider()) and compute is3P from that (const is3P
= provider !== 'firstParty'), then use provider === 'openai' for selecting
customOpusModel, nameEnv, and descEnv (and analogously for Haiku) so all
provider-dependent decisions reference the single provider variable instead of
calling getAPIProvider() multiple times.
- Around line 77-103: In getCustomSonnetOption(), reduce redundant calls to
getAPIProvider() by calling it once into a local const (e.g., const provider =
getAPIProvider()) and then use provider === 'openai' and provider !==
'firstParty' instead of repeated getAPIProvider() calls; update the existing
conditional logic that sets is3P, customSonnetModel, nameEnv, and descEnv to
reference this local provider variable so the function is clearer and avoids
repeated evaluations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f7628743-ec77-4275-9400-3e0696cf4f17
📒 Files selected for processing (13)
CLAUDE.mddocs/plans/openai-compatibility.mdsrc/commands.tssrc/commands/provider.tssrc/components/ConsoleOAuthFlow.tsxsrc/services/api/openai/__tests__/modelMapping.test.tssrc/services/api/openai/modelMapping.tssrc/utils/managedEnvConstants.tssrc/utils/model/__tests__/providers.test.tssrc/utils/model/model.tssrc/utils/model/modelOptions.tssrc/utils/model/modelSupportOverrides.tssrc/utils/model/providers.ts
Comment on lines
+57
to
+72
| // 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.`, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing applyConfigEnvironmentVariables() call when switching to OpenAI with missing env vars.
When OPENAI_API_KEY or OPENAI_BASE_URL are missing (lines 62-71), the code updates settings but returns early without calling applyConfigEnvironmentVariables(). This means any OpenAI env vars that do exist in settings won't be applied to process.env, potentially causing inconsistent state.
Compare to lines 77-86 where applyConfigEnvironmentVariables() is always called for anthropic/openai.
🔧 Suggested fix
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' })
+ applyConfigEnvironmentVariables()
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.`,
}
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/provider.ts` around lines 57 - 72, When switching provider to
'openai' (the block checking if arg === 'openai' using getMergedEnv and
hasKey/hasUrl), ensure applyConfigEnvironmentVariables() is invoked before
returning when env vars are missing: after calling
updateSettingsForSource('userSettings', { modelType: 'openai' }) and before
building/returning the warning response, call applyConfigEnvironmentVariables()
so any existing OpenAI settings are applied to process.env; keep the
early-return behavior but add that call to mirror the later anthropic/openai
flow.
Comment on lines
+88
to
+95
| // 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).` } |
There was a problem hiding this comment.
Deleted env vars may be immediately re-applied by applyConfigEnvironmentVariables().
Lines 89-91 delete CLAUDE_CODE_USE_OPENAI, OPENAI_API_KEY, and OPENAI_BASE_URL from process.env. However, applyConfigEnvironmentVariables() at line 94 uses Object.assign(process.env, filterSettingsEnv(getSettings_DEPRECATED()?.env)) which will re-apply these variables if they exist in settings.env.
The same issue affects lines 79-81 + 85 where cloud provider flags are deleted then potentially restored.
🐛 Potential fix: clear from settings.env as well
} 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
+ // Clear conflicting env vars from settings to prevent re-application
+ const settings = getSettings_DEPRECATED()
+ if (settings?.env) {
+ delete settings.env.CLAUDE_CODE_USE_OPENAI
+ delete settings.env.OPENAI_API_KEY
+ delete settings.env.OPENAI_BASE_URL
+ }
applyConfigEnvironmentVariables()
return { type: 'text', value: `API provider set to ${arg} (via environment variable).` }
}Note: A more robust approach would be to update the settings file via updateSettingsForSource to persist the removal.
📝 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
| // 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).` } | |
| // 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' | |
| // Clear conflicting env vars from settings to prevent re-application | |
| const settings = getSettings_DEPRECATED() | |
| if (settings?.env) { | |
| delete settings.env.CLAUDE_CODE_USE_OPENAI | |
| delete settings.env.OPENAI_API_KEY | |
| delete settings.env.OPENAI_BASE_URL | |
| } | |
| applyConfigEnvironmentVariables() | |
| return { type: 'text', value: `API provider set to ${arg} (via environment variable).` } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/provider.ts` around lines 88 - 95, The deletion of
provider-related env vars from process.env (the calls around delete
process.env.CLAUDE_CODE_USE_OPENAI, OPENAI_API_KEY, OPENAI_BASE_URL and the
earlier deletions) is undone when applyConfigEnvironmentVariables() merges
settings.env via filterSettingsEnv(getSettings_DEPRECATED()?.env); to fix,
remove these keys from the active settings env before re-applying or persist the
change via the settings updater: call updateSettingsForSource (or otherwise
remove the keys from getSettings_DEPRECATED()?.env) for the same keys returned
by getEnvVarForProvider(arg) and the explicit
CLAUDE_CODE_USE_OPENAI/OPENAI_API_KEY/OPENAI_BASE_URL entries so that
applyConfigEnvironmentVariables() won’t re-add them, then call
applyConfigEnvironmentVariables() as before; reference
applyConfigEnvironmentVariables, getEnvVarForProvider, getSettings_DEPRECATED,
filterSettingsEnv, and updateSettingsForSource to locate and implement the
change.
src/components/ConsoleOAuthFlow.tsx
Outdated
Comment on lines
+547
to
+548
| const [inputValue, setInputValue] = useState(() => displayValues[activeField]); | ||
| const [inputCursorOffset, setInputCursorOffset] = useState(() => displayValues[activeField].length); |
There was a problem hiding this comment.
React hooks called inside switch case violate Rules of Hooks.
useState hooks are called conditionally inside the custom_platform switch case (and similarly in openai_chat_api at lines 730-733). React requires hooks to be called in the same order on every render. When oauthStatus.state changes, the number and order of hooks changes, which will cause React to break.
🐛 Suggested approach
Move the useState hooks to the top level of OAuthStatusMessage component, or extract custom_platform and openai_chat_api cases into separate components where the hooks can be called unconditionally:
+function CustomPlatformForm({ oauthStatus, setOAuthStatus, onDone }: { ... }) {
+ const [inputValue, setInputValue] = useState(() => ...);
+ const [inputCursorOffset, setInputCursorOffset] = useState(() => ...);
+ // ... rest of custom_platform logic
+ return <Box>...</Box>;
+}
function OAuthStatusMessage({ ... }): React.ReactNode {
switch (oauthStatus.state) {
case 'custom_platform':
- const [inputValue, setInputValue] = useState(...);
- ...
+ return <CustomPlatformForm oauthStatus={oauthStatus} setOAuthStatus={setOAuthStatus} onDone={onDone} />;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ConsoleOAuthFlow.tsx` around lines 547 - 548, The useState
hooks (inputValue/setInputValue and inputCursorOffset/setInputCursorOffset) are
being called conditionally inside the OAuthStatusMessage switch cases (notably
the custom_platform and openai_chat_api branches), which violates the Rules of
Hooks; move these useState declarations to the top-level of the
OAuthStatusMessage component (so they run unconditionally) or extract the
custom_platform and openai_chat_api branches into their own components (e.g.,
CustomPlatformField and OpenAIChatField) and call the hooks there, ensuring you
keep references to displayValues and activeField to initialize/update inputValue
and inputCursorOffset appropriately.
src/components/ConsoleOAuthFlow.tsx
Outdated
Comment on lines
+631
to
+654
| useKeybinding( | ||
| 'tabs:next', | ||
| () => { | ||
| const idx = FIELDS.indexOf(activeField); | ||
| if (idx < FIELDS.length - 1) { | ||
| setOAuthStatus(buildState(activeField, inputValue, FIELDS[idx + 1])); | ||
| setInputValue(displayValues[FIELDS[idx + 1]!] ?? ''); | ||
| setInputCursorOffset((displayValues[FIELDS[idx + 1]!] ?? '').length); | ||
| } | ||
| }, [activeField, inputValue, displayValues, setOAuthStatus, onDone]) | ||
|
|
||
| const handleEnter = useCallback(() => { | ||
| const idx = FIELDS.indexOf(activeField) | ||
| setOAuthStatus(buildState(activeField, inputValue)) | ||
| if (idx === FIELDS.length - 1) { | ||
| doSave() | ||
| } else { | ||
| const next = FIELDS[idx + 1]! | ||
| setInputValue(displayValues[next] ?? '') | ||
| setInputCursorOffset((displayValues[next] ?? '').length) | ||
| }, | ||
| { context: 'Tabs' }, | ||
| ); | ||
| useKeybinding( | ||
| 'tabs:previous', | ||
| () => { | ||
| const idx = FIELDS.indexOf(activeField); | ||
| if (idx > 0) { | ||
| setOAuthStatus(buildState(activeField, inputValue, FIELDS[idx - 1])); | ||
| setInputValue(displayValues[FIELDS[idx - 1]!] ?? ''); | ||
| setInputCursorOffset((displayValues[FIELDS[idx - 1]!] ?? '').length); | ||
| } | ||
| }, [activeField, inputValue, buildState, doSave, displayValues, setOAuthStatus]) | ||
|
|
||
| useKeybinding( | ||
| 'tabs:next', | ||
| () => { | ||
| const idx = FIELDS.indexOf(activeField) | ||
| if (idx < FIELDS.length - 1) { | ||
| setOAuthStatus(buildState(activeField, inputValue, FIELDS[idx + 1])) | ||
| setInputValue(displayValues[FIELDS[idx + 1]!] ?? '') | ||
| setInputCursorOffset((displayValues[FIELDS[idx + 1]!] ?? '').length) | ||
| } | ||
| }, | ||
| { context: 'Tabs' }, | ||
| ) | ||
| useKeybinding( | ||
| 'tabs:previous', | ||
| () => { | ||
| const idx = FIELDS.indexOf(activeField) | ||
| if (idx > 0) { | ||
| setOAuthStatus(buildState(activeField, inputValue, FIELDS[idx - 1])) | ||
| setInputValue(displayValues[FIELDS[idx - 1]!] ?? '') | ||
| setInputCursorOffset((displayValues[FIELDS[idx - 1]!] ?? '').length) | ||
| } | ||
| }, | ||
| { context: 'Tabs' }, | ||
| ) | ||
| useKeybinding( | ||
| 'confirm:no', | ||
| () => { | ||
| setOAuthStatus({ state: 'idle' }) | ||
| }, | ||
| { context: 'Confirmation' }, | ||
| ) | ||
| }, | ||
| { context: 'Tabs' }, | ||
| ); |
There was a problem hiding this comment.
Multiple useKeybinding hooks inside switch case also violate Rules of Hooks.
These useKeybinding calls (and those at lines 807-837 for the openai case) are only executed when the component renders with that specific state. When the state changes, these hooks disappear or appear, violating hook ordering rules.
Extract these into a dedicated component for the same fix as above.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ConsoleOAuthFlow.tsx` around lines 631 - 654, The two
useKeybinding hook calls inside ConsoleOAuthFlow (the handlers that reference
FIELDS, activeField, setOAuthStatus, buildState, displayValues, setInputValue,
and setInputCursorOffset) violate the Rules of Hooks because they run
conditionally; extract them into a separate child component (e.g.,
OAuthFieldKeybindings) that always mounts and receives props (activeField,
inputValue, displayValues, setOAuthStatus, setInputValue, setInputCursorOffset)
and move both 'tabs:next' and 'tabs:previous' useKeybinding calls into that
component so hooks are unconditionally declared and logic remains identical.
Comment on lines
+25
to
+43
| 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); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Test modifies user's actual ~/.claude/settings.json file.
The beforeAll/afterAll hooks directly modify the user's home directory settings file, which is risky:
- If tests crash or are interrupted, the user's settings could be left corrupted
- Tests are not isolated from the actual environment
- The
afterAllonly restores iforiginalSettingsis truthy—an empty settings file would not be restored
Consider using a test-specific settings path or mocking the file system operations instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/model/__tests__/providers.test.ts` around lines 25 - 43, The tests
currently mutate the real user settings by calling getSettingsPath() inside
beforeAll/afterAll and only restoring when originalSettings is truthy; change
the setup to use an isolated test settings file or mock file-access: replace
calls to getSettingsPath() in the test scope with a test-specific path (e.g.,
create a temp dir/file via os.tmpdir() or fs.mkdtemp) or mock
getSettingsPath/readFileSync/writeFileSync so all I/O targets that temp file;
also ensure originalSettings is tracked distinctly (use undefined vs empty
string) and always restore in afterAll if originalSettings !== undefined (or use
try/finally) so an empty settings file is restored even if tests crash or throw.
Owner
|
貌似你的代码格式化有些问题, 导致好多 diff. 然后看看 code review 的 bug 提示修复一下 |
Contributor
Author
claude code 说因为他定位代码位置有问题所以格式化一下很“合理” 这次只格式化了ConsoleOAuthFlow.tsx 上一次他把整个项目代码全格式化了一边 辣眼睛 |
Owner
|
哎, 我的锅, 当时就应该全部格式化的... |
added 2 commits
April 5, 2026 11:59
Contributor
Author
这下应该没问题了 |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/ConsoleOAuthFlow.tsx (1)
847-848:⚠️ Potential issue | 🟡 MinorSame misleading comment issue as the custom_platform case.
This has the same mismatch between the comment ("Schedule onDone after state update") and the synchronous
void onDone()implementation. Consider applying the same fix pattern if deferral is the intended behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ConsoleOAuthFlow.tsx` around lines 847 - 848, The comment in ConsoleOAuthFlow.tsx incorrectly says "Schedule onDone after state update" while the code calls onDone synchronously; update the call to actually defer execution (e.g., use setTimeout(..., 0) or queueMicrotask) so onDone is invoked after the component state update completes — change the synchronous void onDone() call to a deferred invocation for the onDone handler in the ConsoleOAuthFlow component (mirror the same fix used for the custom_platform case).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ConsoleOAuthFlow.tsx`:
- Around line 650-651: The inline comment is inaccurate because `void onDone()`
calls onDone synchronously; if the intent in ConsoleOAuthFlow is to let the
updated success state render before running the callback, change the call site
that currently calls `onDone()` (the onDone invocation) to defer execution (for
example wrap it with setTimeout(() => onDone(), 0) or use queueMicrotask(() =>
onDone())) so the callback runs after the state update and render rather than
synchronously.
---
Duplicate comments:
In `@src/components/ConsoleOAuthFlow.tsx`:
- Around line 847-848: The comment in ConsoleOAuthFlow.tsx incorrectly says
"Schedule onDone after state update" while the code calls onDone synchronously;
update the call to actually defer execution (e.g., use setTimeout(..., 0) or
queueMicrotask) so onDone is invoked after the component state update completes
— change the synchronous void onDone() call to a deferred invocation for the
onDone handler in the ConsoleOAuthFlow component (mirror the same fix used for
the custom_platform case).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 13508814-a2c2-49e3-ab4c-4d683638f222
📒 Files selected for processing (1)
src/components/ConsoleOAuthFlow.tsx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/ConsoleOAuthFlow.tsx (1)
621-650:⚠️ Potential issue | 🟠 MajorValidate
base_urlbefore saving it.Both save paths persist arbitrary strings straight into
ANTHROPIC_BASE_URL/OPENAI_BASE_URL. The PR screenshot’s later"cannot be parsed as a URL"failure is exactly what happens when this form accepts an invalid base URL. Please reject malformed URLs here with an actionable save-time error instead of letting the next request crash.Also applies to: 817-846
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ConsoleOAuthFlow.tsx` around lines 621 - 650, Validate the base_url before persisting it in doSave: parse finalVals.base_url (and the analogous field in the other save path) with a URL parse (try/catch using the URL constructor) and if it fails setOAuthStatus to an error state with an actionable message (e.g., "Invalid base URL: please enter a full URL including protocol") and do not call updateSettingsForSource, do not populate env or process.env, and do not call onDone(); if the URL is valid continue with the existing updateSettingsForSource and process.env assignment. Reference symbols: doSave, finalVals, inputValue, displayValues, updateSettingsForSource, setOAuthStatus, ANTHROPIC_BASE_URL (and OPENAI_BASE_URL for the other save path).
♻️ Duplicate comments (1)
src/components/ConsoleOAuthFlow.tsx (1)
647-650:⚠️ Potential issue | 🟡 Minor
onDone()still runs before the success state can render.
setOAuthStatus({ state: 'success' })is immediately followed byonDone(), so these branches can unmount before the success UI appears. The setup-token path already defersonDone; these save paths should do the same.Suggested fix
setOAuthStatus({ state: 'success' }) - void onDone() + setTimeout(() => onDone(), 0)Apply the same change in both save branches.
Also applies to: 843-846
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ConsoleOAuthFlow.tsx` around lines 647 - 650, The save branches call setOAuthStatus({ state: 'success' }) then immediately invoke onDone(), causing the success UI to never render; update both save branches (the ones that setOAuthStatus to 'success' and then call void onDone()) to defer invoking onDone() the same way the setup-token path does (e.g., wrap the onDone() call in a short async deferral such as setTimeout/ requestAnimationFrame or the existing helper used in setup-token) so state can render before unmounting. Ensure you modify the occurrences where setOAuthStatus and void onDone() are paired (including the branch that assigns process.env from env and the other save branch referenced) to match the setup-token behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/ConsoleOAuthFlow.tsx`:
- Around line 621-650: Validate the base_url before persisting it in doSave:
parse finalVals.base_url (and the analogous field in the other save path) with a
URL parse (try/catch using the URL constructor) and if it fails setOAuthStatus
to an error state with an actionable message (e.g., "Invalid base URL: please
enter a full URL including protocol") and do not call updateSettingsForSource,
do not populate env or process.env, and do not call onDone(); if the URL is
valid continue with the existing updateSettingsForSource and process.env
assignment. Reference symbols: doSave, finalVals, inputValue, displayValues,
updateSettingsForSource, setOAuthStatus, ANTHROPIC_BASE_URL (and OPENAI_BASE_URL
for the other save path).
---
Duplicate comments:
In `@src/components/ConsoleOAuthFlow.tsx`:
- Around line 647-650: The save branches call setOAuthStatus({ state: 'success'
}) then immediately invoke onDone(), causing the success UI to never render;
update both save branches (the ones that setOAuthStatus to 'success' and then
call void onDone()) to defer invoking onDone() the same way the setup-token path
does (e.g., wrap the onDone() call in a short async deferral such as setTimeout/
requestAnimationFrame or the existing helper used in setup-token) so state can
render before unmounting. Ensure you modify the occurrences where setOAuthStatus
and void onDone() are paired (including the branch that assigns process.env from
env and the other save branch referenced) to match the setup-token behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5e500e28-6fab-4eb4-a088-72d5ece73eda
📒 Files selected for processing (1)
src/components/ConsoleOAuthFlow.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/components/ConsoleOAuthFlow.tsx (1)
580-721:⚠️ Potential issue | 🔴 CriticalThese setup branches still break the Rules of Hooks.
useState,useCallback,useKeybinding, anduseTerminalSizeare still declared only inside thecustom_platform/openai_chat_apiswitch branches. WhenoauthStatus.statechanges, the hook order changes too, which can break the component at runtime.Minimal direction
function OAuthStatusMessage(props: OAuthStatusMessageProps): React.ReactNode { switch (props.oauthStatus.state) { case 'custom_platform': - const [inputValue, setInputValue] = useState(...) - ... - return ... + return <CustomPlatformSetup {...props} oauthStatus={props.oauthStatus} /> case 'openai_chat_api': - const [openaiInputValue, setOpenaiInputValue] = useState(...) - ... - return ... + return <OpenAIChatApiSetup {...props} oauthStatus={props.oauthStatus} /> } }Also applies to: 806-955
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ConsoleOAuthFlow.tsx` around lines 580 - 721, The hooks (useState/useCallback/useKeybinding/useTerminalSize) must not be declared conditionally inside the 'custom_platform'/'openai_chat_api' branches because changing oauthStatus.state reorders hooks; move declarations for inputValue, inputCursorOffset, buildState, switchTo, doSave, handleEnter, all useKeybinding calls, and const columns = useTerminalSize().columns - 20 to the top-level of the component (always executed), then have their logic read/write based on oauthStatus/state (or use refs) so they only become active when state === 'custom_platform' or 'openai_chat_api'; keep references to setOAuthStatus, displayValues, activeField, baseUrl, apiKey, haikuModel, sonnetModel, opusModel in the callbacks' dependency arrays and ensure any state initializers use displayValues[activeField] safely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ConsoleOAuthFlow.tsx`:
- Around line 630-641: The setOAuthStatus call is resetting the entire form on
URL validation error; instead populate the toRetry object with the existing
values (e.g., reuse finalVals.base_url, finalVals.apiKey, finalVals.haikuModel,
finalVals.sonnetModel, finalVals.opusModel and finalVals.activeField) rather
than blank strings so only base_url is highlighted for correction; update the
same change in the openai_chat_api branch where a similar setOAuthStatus({...
toRetry: {...}}) reset occurs (also apply same fix in the other occurrence
around lines 849-860).
- Around line 534-536: Prefill logic for haikuModel/sonnetModel/opusModel only
reads OPENAI_DEFAULT_* and thus ignores legacy ANTHROPIC_DEFAULT_* fallbacks;
update the prefilling to try OPENAI_DEFAULT_HAIKU_MODEL (etc.) then fallback to
ANTHROPIC_DEFAULT_HAIKU_MODEL (etc.) before finally using '' so existing
installs with only legacy overrides still show values—apply this change to the
haikuModel, sonnetModel and opusModel assignments in ConsoleOAuthFlow.tsx.
---
Duplicate comments:
In `@src/components/ConsoleOAuthFlow.tsx`:
- Around line 580-721: The hooks
(useState/useCallback/useKeybinding/useTerminalSize) must not be declared
conditionally inside the 'custom_platform'/'openai_chat_api' branches because
changing oauthStatus.state reorders hooks; move declarations for inputValue,
inputCursorOffset, buildState, switchTo, doSave, handleEnter, all useKeybinding
calls, and const columns = useTerminalSize().columns - 20 to the top-level of
the component (always executed), then have their logic read/write based on
oauthStatus/state (or use refs) so they only become active when state ===
'custom_platform' or 'openai_chat_api'; keep references to setOAuthStatus,
displayValues, activeField, baseUrl, apiKey, haikuModel, sonnetModel, opusModel
in the callbacks' dependency arrays and ensure any state initializers use
displayValues[activeField] safely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b3dd181b-c4a1-4f08-a6dc-9576ce21f660
📒 Files selected for processing (1)
src/components/ConsoleOAuthFlow.tsx
Comment on lines
+534
to
+536
| haikuModel: process.env.OPENAI_DEFAULT_HAIKU_MODEL ?? '', | ||
| sonnetModel: process.env.OPENAI_DEFAULT_SONNET_MODEL ?? '', | ||
| opusModel: process.env.OPENAI_DEFAULT_OPUS_MODEL ?? '', |
There was a problem hiding this comment.
Mirror runtime fallback when prefilling OpenAI model fields.
The runtime still falls back from OPENAI_DEFAULT_* to ANTHROPIC_DEFAULT_* for OpenAI model resolution, but this form now reads only the new keys. Users upgrading with only legacy overrides set will see blank fields even though those overrides are still active.
💡 Suggested fix
- haikuModel: process.env.OPENAI_DEFAULT_HAIKU_MODEL ?? '',
- sonnetModel: process.env.OPENAI_DEFAULT_SONNET_MODEL ?? '',
- opusModel: process.env.OPENAI_DEFAULT_OPUS_MODEL ?? '',
+ haikuModel:
+ process.env.OPENAI_DEFAULT_HAIKU_MODEL ||
+ process.env.ANTHROPIC_DEFAULT_HAIKU_MODEL ||
+ '',
+ sonnetModel:
+ process.env.OPENAI_DEFAULT_SONNET_MODEL ||
+ process.env.ANTHROPIC_DEFAULT_SONNET_MODEL ||
+ '',
+ opusModel:
+ process.env.OPENAI_DEFAULT_OPUS_MODEL ||
+ process.env.ANTHROPIC_DEFAULT_OPUS_MODEL ||
+ '',📝 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
| haikuModel: process.env.OPENAI_DEFAULT_HAIKU_MODEL ?? '', | |
| sonnetModel: process.env.OPENAI_DEFAULT_SONNET_MODEL ?? '', | |
| opusModel: process.env.OPENAI_DEFAULT_OPUS_MODEL ?? '', | |
| haikuModel: | |
| process.env.OPENAI_DEFAULT_HAIKU_MODEL || | |
| process.env.ANTHROPIC_DEFAULT_HAIKU_MODEL || | |
| '', | |
| sonnetModel: | |
| process.env.OPENAI_DEFAULT_SONNET_MODEL || | |
| process.env.ANTHROPIC_DEFAULT_SONNET_MODEL || | |
| '', | |
| opusModel: | |
| process.env.OPENAI_DEFAULT_OPUS_MODEL || | |
| process.env.ANTHROPIC_DEFAULT_OPUS_MODEL || | |
| '', |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ConsoleOAuthFlow.tsx` around lines 534 - 536, Prefill logic
for haikuModel/sonnetModel/opusModel only reads OPENAI_DEFAULT_* and thus
ignores legacy ANTHROPIC_DEFAULT_* fallbacks; update the prefilling to try
OPENAI_DEFAULT_HAIKU_MODEL (etc.) then fallback to ANTHROPIC_DEFAULT_HAIKU_MODEL
(etc.) before finally using '' so existing installs with only legacy overrides
still show values—apply this change to the haikuModel, sonnetModel and opusModel
assignments in ConsoleOAuthFlow.tsx.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes / UX
Tests