Skip to content

尝试provider(api)指令切换模型类型#122

Closed
2228293026 wants to merge 15 commits intoclaude-code-best:mainfrom
2228293026:main
Closed

尝试provider(api)指令切换模型类型#122
2228293026 wants to merge 15 commits intoclaude-code-best:mainfrom
2228293026:main

Conversation

@2228293026
Copy link
Copy Markdown
Contributor

@2228293026 2228293026 commented Apr 4, 2026

image

Summary by CodeRabbit

  • New Features
    • Added a provider (alias: api) command to view, switch, or unset the active API provider (Anthropic, OpenAI, Bedrock, Vertex, Foundry).
    • Switching validates inputs and warns if required credentials are missing; unsetting clears related configuration.
  • Improvements
    • Provider selection now prefers persisted settings before falling back to environment-based detection.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 4, 2026

Important

Review skipped

Too many files!

This PR contains 299 files, which is 149 over the limit of 150.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 594f003e-0f2a-4f7f-a08e-373a95359091

📥 Commits

Reviewing files that changed from the base of the PR and between ec5dfed and 130e4fb.

📒 Files selected for processing (299)
  • CLAUDE.md
  • docs/plans/openai-compatibility.md
  • src/QueryEngine.ts
  • src/assistant/AssistantSessionChooser.ts
  • src/assistant/gate.ts
  • src/assistant/index.ts
  • src/assistant/sessionDiscovery.ts
  • src/bootstrap/src/entrypoints/agentSdkTypes.ts
  • src/bootstrap/src/tools/AgentTool/agentColorManager.ts
  • src/bootstrap/src/types/hooks.ts
  • src/bootstrap/src/types/ids.ts
  • src/bootstrap/src/utils/crypto.ts
  • src/bootstrap/src/utils/model/model.ts
  • src/bootstrap/src/utils/model/modelStrings.ts
  • src/bootstrap/src/utils/settings/constants.ts
  • src/bootstrap/src/utils/settings/settingsCache.ts
  • src/bootstrap/src/utils/settings/types.ts
  • src/bootstrap/src/utils/signal.ts
  • src/bootstrap/state.ts
  • src/bridge/bridgeMessaging.ts
  • src/bridge/inboundMessages.ts
  • src/bridge/remoteBridgeCore.ts
  • src/bridge/replBridge.ts
  • src/bridge/src/entrypoints/sdk/controlTypes.ts
  • src/bridge/webhookSanitizer.ts
  • src/buddy/CompanionSprite.tsx
  • src/buddy/useBuddyNotification.tsx
  • src/cli/bg.ts
  • src/cli/handlers/ant.ts
  • src/cli/handlers/auth.ts
  • src/cli/handlers/mcp.tsx
  • src/cli/handlers/templateJobs.ts
  • src/cli/handlers/util.tsx
  • src/cli/print.ts
  • src/cli/rollback.ts
  • src/cli/src/QueryEngine.ts
  • src/cli/src/cli/handlers/auth.ts
  • src/cli/src/cli/remoteIO.ts
  • src/cli/src/cli/structuredIO.ts
  • src/cli/src/commands/context/context-noninteractive.ts
  • src/cli/src/entrypoints/agentSdkTypes.ts
  • src/cli/src/entrypoints/sdk/controlSchemas.ts
  • src/cli/src/entrypoints/sdk/controlTypes.ts
  • src/cli/src/hooks/useCanUseTool.ts
  • src/cli/src/services/PromptSuggestion/promptSuggestion.ts
  • src/cli/src/services/analytics/growthbook.ts
  • src/cli/src/services/analytics/index.ts
  • src/cli/src/services/api/grove.ts
  • src/cli/src/services/api/logging.ts
  • src/cli/src/services/claudeAiLimits.ts
  • src/cli/src/services/mcp/auth.ts
  • src/cli/src/services/mcp/channelAllowlist.ts
  • src/cli/src/services/mcp/channelNotification.ts
  • src/cli/src/services/mcp/client.ts
  • src/cli/src/services/mcp/config.ts
  • src/cli/src/services/mcp/elicitationHandler.ts
  • src/cli/src/services/mcp/mcpStringUtils.ts
  • src/cli/src/services/mcp/types.ts
  • src/cli/src/services/mcp/utils.ts
  • src/cli/src/services/mcp/vscodeSdkMcp.ts
  • src/cli/src/services/oauth/index.ts
  • src/cli/src/services/policyLimits/index.ts
  • src/cli/src/services/remoteManagedSettings/index.ts
  • src/cli/src/services/settingsSync/index.ts
  • src/cli/src/state/AppStateStore.ts
  • src/cli/src/state/onChangeAppState.ts
  • src/cli/src/tools.ts
  • src/cli/src/utils/abortController.ts
  • src/cli/src/utils/array.ts
  • src/cli/src/utils/auth.ts
  • src/cli/src/utils/autoUpdater.ts
  • src/cli/src/utils/awsAuthStatusManager.ts
  • src/cli/src/utils/betas.ts
  • src/cli/src/utils/cleanupRegistry.ts
  • src/cli/src/utils/combinedAbortSignal.ts
  • src/cli/src/utils/commandLifecycle.ts
  • src/cli/src/utils/commitAttribution.ts
  • src/cli/src/utils/completionCache.ts
  • src/cli/src/utils/config.ts
  • src/cli/src/utils/conversationRecovery.ts
  • src/cli/src/utils/cwd.ts
  • src/cli/src/utils/debug.ts
  • src/cli/src/utils/diagLogs.ts
  • src/cli/src/utils/doctorDiagnostic.ts
  • src/cli/src/utils/effort.ts
  • src/cli/src/utils/errors.ts
  • src/cli/src/utils/fastMode.ts
  • src/cli/src/utils/fileHistory.ts
  • src/cli/src/utils/filePersistence/filePersistence.ts
  • src/cli/src/utils/fileStateCache.ts
  • src/cli/src/utils/forkedAgent.ts
  • src/cli/src/utils/generators.ts
  • src/cli/src/utils/gracefulShutdown.ts
  • src/cli/src/utils/headlessProfiler.ts
  • src/cli/src/utils/hooks.ts
  • src/cli/src/utils/hooks/AsyncHookRegistry.ts
  • src/cli/src/utils/hooks/hookEvents.ts
  • src/cli/src/utils/idleTimeout.ts
  • src/cli/src/utils/json.ts
  • src/cli/src/utils/localInstaller.ts
  • src/cli/src/utils/log.ts
  • src/cli/src/utils/messageQueueManager.ts
  • src/cli/src/utils/messages.ts
  • src/cli/src/utils/messages/mappers.ts
  • src/cli/src/utils/model/model.ts
  • src/cli/src/utils/model/modelOptions.ts
  • src/cli/src/utils/model/modelStrings.ts
  • src/cli/src/utils/model/providers.ts
  • src/cli/src/utils/nativeInstaller/index.ts
  • src/cli/src/utils/nativeInstaller/packageManagers.ts
  • src/cli/src/utils/path.ts
  • src/cli/src/utils/permissions/PermissionPromptToolResultSchema.ts
  • src/cli/src/utils/permissions/PermissionResult.ts
  • src/cli/src/utils/permissions/permissionSetup.ts
  • src/cli/src/utils/permissions/permissions.ts
  • src/cli/src/utils/plugins/pluginIdentifier.ts
  • src/cli/src/utils/process.ts
  • src/cli/src/utils/queryContext.ts
  • src/cli/src/utils/queryHelpers.ts
  • src/cli/src/utils/queryProfiler.ts
  • src/cli/src/utils/sandbox/sandbox-adapter.ts
  • src/cli/src/utils/semver.ts
  • src/cli/src/utils/sessionRestore.ts
  • src/cli/src/utils/sessionStart.ts
  • src/cli/src/utils/sessionState.ts
  • src/cli/src/utils/sessionStorage.ts
  • src/cli/src/utils/sessionTitle.ts
  • src/cli/src/utils/sessionUrl.ts
  • src/cli/src/utils/sideQuestion.ts
  • src/cli/src/utils/stream.ts
  • src/cli/src/utils/streamJsonStdoutGuard.ts
  • src/cli/src/utils/streamlinedTransform.ts
  • src/cli/src/utils/thinking.ts
  • src/cli/src/utils/toolPool.ts
  • src/cli/src/utils/uuid.ts
  • src/cli/src/utils/workloadContext.ts
  • src/cli/structuredIO.ts
  • src/cli/transports/Transport.ts
  • src/cli/transports/ccrClient.ts
  • src/cli/transports/src/entrypoints/sdk/controlTypes.ts
  • src/commands/add-dir/add-dir.tsx
  • src/commands/agents/agents.tsx
  • src/commands/assistant/assistant.ts
  • src/commands/branch/branch.ts
  • src/commands/bridge/bridge.tsx
  • src/commands/btw/btw.tsx
  • src/commands/chrome/chrome.tsx
  • src/commands/compact/src/bootstrap/state.ts
  • src/commands/config/config.tsx
  • src/commands/context/context.tsx
  • src/commands/copy/copy.tsx
  • src/commands/desktop/desktop.tsx
  • src/commands/diff/diff.tsx
  • src/commands/doctor/doctor.tsx
  • src/commands/effort/effort.tsx
  • src/commands/exit/exit.tsx
  • src/commands/export/export.tsx
  • src/commands/extra-usage/extra-usage.tsx
  • src/commands/fast/fast.tsx
  • src/commands/feedback/feedback.tsx
  • src/commands/fork/index.ts
  • src/commands/help/help.tsx
  • src/commands/hooks/hooks.tsx
  • src/commands/ide/ide.tsx
  • src/commands/ide/src/services/analytics/index.ts
  • src/commands/insights.ts
  • src/commands/install-github-app/ApiKeyStep.tsx
  • src/commands/install-github-app/CheckExistingSecretStep.tsx
  • src/commands/install-github-app/CheckGitHubStep.tsx
  • src/commands/install-github-app/ChooseRepoStep.tsx
  • src/commands/install-github-app/CreatingStep.tsx
  • src/commands/install-github-app/ErrorStep.tsx
  • src/commands/install-github-app/ExistingWorkflowStep.tsx
  • src/commands/install-github-app/InstallAppStep.tsx
  • src/commands/install-github-app/OAuthFlowStep.tsx
  • src/commands/install-github-app/SuccessStep.tsx
  • src/commands/install-github-app/WarningsStep.tsx
  • src/commands/install-github-app/install-github-app.tsx
  • src/commands/install-github-app/src/components/CustomSelect/index.ts
  • src/commands/install-github-app/src/services/analytics/index.ts
  • src/commands/install-github-app/src/utils/config.ts
  • src/commands/install-github-app/types.ts
  • src/commands/install.tsx
  • src/commands/login/login.tsx
  • src/commands/logout/logout.tsx
  • src/commands/mcp/mcp.tsx
  • src/commands/memory/memory.tsx
  • src/commands/mobile/mobile.tsx
  • src/commands/model/model.tsx
  • src/commands/output-style/output-style.tsx
  • src/commands/passes/passes.tsx
  • src/commands/peers/index.ts
  • src/commands/permissions/permissions.tsx
  • src/commands/plan/plan.tsx
  • src/commands/plugin/AddMarketplace.tsx
  • src/commands/plugin/BrowseMarketplace.tsx
  • src/commands/plugin/DiscoverPlugins.tsx
  • src/commands/plugin/ManageMarketplaces.tsx
  • src/commands/plugin/ManagePlugins.tsx
  • src/commands/plugin/PluginErrors.tsx
  • src/commands/plugin/PluginOptionsDialog.tsx
  • src/commands/plugin/PluginOptionsFlow.tsx
  • src/commands/plugin/PluginSettings.tsx
  • src/commands/plugin/PluginTrustWarning.tsx
  • src/commands/plugin/UnifiedInstalledCell.tsx
  • src/commands/plugin/ValidatePlugin.tsx
  • src/commands/plugin/__tests__/parseArgs.test.ts
  • src/commands/plugin/index.tsx
  • src/commands/plugin/plugin.tsx
  • src/commands/plugin/pluginDetailsHelpers.tsx
  • src/commands/plugin/src/services/analytics/index.ts
  • src/commands/plugin/types.ts
  • src/commands/plugin/unifiedTypes.ts
  • src/commands/privacy-settings/privacy-settings.tsx
  • src/commands/provider.ts
  • src/commands/rate-limit-options/rate-limit-options.tsx
  • src/commands/remote-env/remote-env.tsx
  • src/commands/remote-setup/remote-setup.tsx
  • src/commands/rename/generateSessionName.ts
  • src/commands/reset-limits/index.ts
  • src/commands/resume/resume.tsx
  • src/commands/review/UltrareviewOverageDialog.tsx
  • src/commands/review/reviewRemote.ts
  • src/commands/review/ultrareviewCommand.tsx
  • src/commands/sandbox-toggle/sandbox-toggle.tsx
  • src/commands/session/session.tsx
  • src/commands/skills/skills.tsx
  • src/commands/src/commands.ts
  • src/commands/src/services/analytics/index.ts
  • src/commands/stats/stats.tsx
  • src/commands/status/status.tsx
  • src/commands/statusline.tsx
  • src/commands/tag/tag.tsx
  • src/commands/tasks/tasks.tsx
  • src/commands/terminalSetup/src/utils/theme.ts
  • src/commands/terminalSetup/terminalSetup.tsx
  • src/commands/theme/theme.tsx
  • src/commands/thinkback/thinkback.tsx
  • src/commands/ultraplan.tsx
  • src/commands/upgrade/upgrade.tsx
  • src/commands/usage/usage.tsx
  • src/commands/workflows/index.ts
  • src/components/AgentProgressLine.tsx
  • src/components/App.tsx
  • src/components/ApproveApiKey.tsx
  • src/components/AutoModeOptInDialog.tsx
  • src/components/AutoUpdater.tsx
  • src/components/AutoUpdaterWrapper.tsx
  • src/components/AwsAuthStatusBox.tsx
  • src/components/BaseTextInput.tsx
  • src/components/BashModeProgress.tsx
  • src/components/BridgeDialog.tsx
  • src/components/BypassPermissionsModeDialog.tsx
  • src/components/ChannelDowngradeDialog.tsx
  • src/components/ClaudeCodeHint/PluginHintMenu.tsx
  • src/components/ClaudeInChromeOnboarding.tsx
  • src/components/ClaudeMdExternalIncludesDialog.tsx
  • src/components/ClickableImageRef.tsx
  • src/components/CompactSummary.tsx
  • src/components/ConfigurableShortcutHint.tsx
  • src/components/ConsoleOAuthFlow.tsx
  • src/components/ContextSuggestions.tsx
  • src/components/ContextVisualization.tsx
  • src/components/CoordinatorAgentStatus.tsx
  • src/components/CostThresholdDialog.tsx
  • src/components/CtrlOToExpand.tsx
  • src/components/CustomSelect/SelectMulti.tsx
  • src/components/CustomSelect/select-input-option.tsx
  • src/components/CustomSelect/select-option.tsx
  • src/components/CustomSelect/select.tsx
  • src/components/DesktopHandoff.tsx
  • src/components/DesktopUpsell/DesktopUpsellStartup.tsx
  • src/components/DevBar.tsx
  • src/components/DevChannelsDialog.tsx
  • src/components/DiagnosticsDisplay.tsx
  • src/components/EffortCallout.tsx
  • src/components/ExitFlow.tsx
  • src/components/ExportDialog.tsx
  • src/components/FallbackToolUseErrorMessage.tsx
  • src/components/FallbackToolUseRejectedMessage.tsx
  • src/components/FastIcon.tsx
  • src/components/Feedback.tsx
  • src/components/FeedbackSurvey/FeedbackSurvey.tsx
  • src/components/FeedbackSurvey/FeedbackSurveyView.tsx
  • src/components/FeedbackSurvey/TranscriptSharePrompt.tsx
  • src/components/FeedbackSurvey/src/hooks/useDynamicConfig.ts
  • src/components/FeedbackSurvey/src/services/analytics/config.ts
  • src/components/FeedbackSurvey/src/services/analytics/growthbook.ts
  • src/components/FeedbackSurvey/src/services/analytics/index.ts
  • src/components/FeedbackSurvey/useFeedbackSurvey.tsx
  • src/components/FeedbackSurvey/useFrustrationDetection.ts
  • src/components/FeedbackSurvey/useMemorySurvey.tsx
  • src/components/FeedbackSurvey/usePostCompactSurvey.tsx
  • src/components/FeedbackSurvey/useSurveyState.tsx
  • src/components/FeedbackSurvey/utils.ts
  • src/components/FileEditToolDiff.tsx
  • src/components/FileEditToolUpdatedMessage.tsx
  • src/components/FileEditToolUseRejectedMessage.tsx
  • src/components/FilePathLink.tsx

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a new local provider/api command to view or switch the active API provider and updates provider detection to prefer persisted modelType for cloud providers (bedrock, vertex, foundry) before falling back to env-var logic.

Changes

Cohort / File(s) Summary
Command registration
src/commands.ts
Imported and registered the new provider command in the main COMMANDS list.
Provider command implementation
src/commands/provider.ts
New local command implementing viewing/unsetting/switching providers (anthropic, openai, bedrock, vertex, foundry), managing userSettings.modelType, deleting/setting provider-specific env toggles, validating inputs, and applying configuration env vars.
Provider detection
src/utils/model/providers.ts
Adjusted getAPIProvider() to prioritize getInitialSettings().modelType for bedrock/vertex/foundry before falling back to environment-variable-based detection.

Sequence Diagram

sequenceDiagram
    participant User
    participant ProviderCmd as Provider Command
    participant ProviderUtil as Provider Utility
    participant Settings as Settings Store
    participant ConfigApp as Config Applier

    User->>ProviderCmd: invoke `provider` / `api` (arg)
    ProviderCmd->>ProviderUtil: getAPIProvider()
    ProviderUtil->>Settings: read getInitialSettings().modelType / env
    Settings-->>ProviderUtil: return current provider
    ProviderUtil-->>ProviderCmd: current provider
    alt unset
        ProviderCmd->>Settings: clear userSettings.modelType
        ProviderCmd->>Settings: remove provider env toggles
        ProviderCmd-->>User: confirm unset (env fallback)
    else switch to provider
        ProviderCmd->>Settings: set userSettings.modelType (or env toggle)
        ProviderCmd->>ConfigApp: applyConfigEnvironmentVariables()
        ConfigApp->>Settings: write env/config as needed
        ConfigApp-->>ProviderCmd: applied
        ProviderCmd-->>User: confirm new provider (or warn missing vars)
    end
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hop to change the API today,
Flip env flags, or set the model's way,
Anthropic, OpenAI, cloud — choose with cheer,
Settings adjusted, confirmations near,
A joyful hop — the provider's clear!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title is in Chinese and uses a generic verb 'try' (尝试) rather than clearly describing what was implemented. It refers to the provider/api command but lacks clarity about the actual primary change. Revise the title to be more specific and clear in English, such as 'Add provider command for switching API providers' or 'Implement provider command to manage model type configuration'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
src/commands/provider.ts (2)

57-72: Consider blocking switch when required env vars are missing.

When OPENAI_API_KEY or OPENAI_BASE_URL is missing, the command still sets modelType: 'openai' and returns. This leaves the system in a state where subsequent API calls will fail.

Also note: the early return skips applyConfigEnvironmentVariables() (line 85), so any provider-specific env vars from settings.env won't be applied.

♻️ Option: Apply config env vars before checking
   if (arg === 'openai') {
+    // Apply settings.env first so user-configured vars are available
+    applyConfigEnvironmentVariables()
     const mergedEnv = getMergedEnv()
     const hasKey = !!mergedEnv.OPENAI_API_KEY
     const hasUrl = !!mergedEnv.OPENAI_BASE_URL
     if (!hasKey || !hasUrl) {
       updateSettingsForSource('userSettings', { modelType: 'openai' })
🤖 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, The current logic in the
provider switch (when arg === 'openai') sets
updateSettingsForSource('userSettings', { modelType: 'openai' }) and returns
even if getMergedEnv() is missing OPENAI_API_KEY or OPENAI_BASE_URL, leaving the
app in a broken state and skipping applyConfigEnvironmentVariables(); fix by
applying provider-specific config env vars first (call
applyConfigEnvironmentVariables() before reading getMergedEnv()), then validate
mergedEnv for OPENAI_API_KEY and OPENAI_BASE_URL and only call
updateSettingsForSource('userSettings', { modelType: 'openai' }) if both are
present; if missing, return the warning without changing settings and avoid the
early return that prevents env application.

1-6: Use src/* path alias for imports.

Per coding guidelines, TypeScript files should use src/* path alias for imports.

♻️ 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 { 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, Update the import statements in
provider.ts to use the project path alias instead of relative paths: replace
imports that bring in Command, LocalCommandCall, getAPIProvider,
updateSettingsForSource, getSettings_DEPRECATED, and
applyConfigEnvironmentVariables so they import from the corresponding "src/..."
modules (e.g., src/commands, src/types/command, src/utils/model/providers,
src/utils/settings/settings, src/utils/managedEnv) while preserving the same
named symbols; this ensures TypeScript path-alias rules are followed without
changing the imported identifiers or behavior.
🤖 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 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.
- Around line 87-98: The branch that handles cloud providers is both deleting
global OpenAI env vars and mutating saved settings; change it to only remove the
CLAUDE_CODE_USE_OPENAI flag from process.env (do not delete OPENAI_API_KEY or
OPENAI_BASE_URL) and stop overwriting user settings via
updateSettingsForSource('userSettings', { modelType: 'anthropic' }); instead
leave settings.json untouched (or only set modelType when a validation error
forces a fallback, with clear comment/documentation). Ensure
applyConfigEnvironmentVariables() is still called so provider-specific envs are
applied, and update any log/comments to reflect that only CLAUDE_CODE_USE_OPENAI
is cleared and settings.json is not modified by the cloud-provider branch.

---

Nitpick comments:
In `@src/commands/provider.ts`:
- Around line 57-72: The current logic in the provider switch (when arg ===
'openai') sets updateSettingsForSource('userSettings', { modelType: 'openai' })
and returns even if getMergedEnv() is missing OPENAI_API_KEY or OPENAI_BASE_URL,
leaving the app in a broken state and skipping
applyConfigEnvironmentVariables(); fix by applying provider-specific config env
vars first (call applyConfigEnvironmentVariables() before reading
getMergedEnv()), then validate mergedEnv for OPENAI_API_KEY and OPENAI_BASE_URL
and only call updateSettingsForSource('userSettings', { modelType: 'openai' })
if both are present; if missing, return the warning without changing settings
and avoid the early return that prevents env application.
- Around line 1-6: Update the import statements in provider.ts to use the
project path alias instead of relative paths: replace imports that bring in
Command, LocalCommandCall, getAPIProvider, updateSettingsForSource,
getSettings_DEPRECATED, and applyConfigEnvironmentVariables so they import from
the corresponding "src/..." modules (e.g., src/commands, src/types/command,
src/utils/model/providers, src/utils/settings/settings, src/utils/managedEnv)
while preserving the same named symbols; this ensures TypeScript path-alias
rules are followed without changing the imported identifiers or behavior.
🪄 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: 1d8a510d-ee1a-420e-b583-0615d32c2e6e

📥 Commits

Reviewing files that changed from the base of the PR and between 5b1a52b and b5d1dbc.

📒 Files selected for processing (3)
  • src/commands.ts
  • src/commands/provider.ts
  • src/utils/model/providers.ts

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

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.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

♻️ Duplicate comments (2)
src/commands/provider.ts (2)

22-29: ⚠️ Potential issue | 🟡 Minor

Fix getMergedEnv typing; env values are nullable in TS.

process.env values are string | undefined, so returning Record<string, string> is unsound.

🛠️ Proposed 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
 }
In TypeScript definitions for Node.js/Bun compatibility, what is the value type of `process.env` (`ProcessEnv`)?
🤖 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
declares a return type of Record<string, string> but copies from process.env
whose values are string | undefined, which is unsound; change the function
signature and internal typing to return Record<string, string | undefined> (or
alternatively coerce/filter undefined values to strings if you want to guarantee
no undefineds), update the merged variable to reflect ProcessEnv typing, and
ensure any callers of getMergedEnv handle the string | undefined value; refer to
getMergedEnv, getSettings_DEPRECATED, merged and process.env when making the
change.

89-91: ⚠️ Potential issue | 🟠 Major

Do not delete user OpenAI credentials when selecting cloud providers.

Removing OPENAI_API_KEY/OPENAI_BASE_URL is destructive and can break other tooling in the same session.

♻️ Proposed fix
     delete process.env.CLAUDE_CODE_USE_OPENAI
-    delete process.env.OPENAI_API_KEY
-    delete process.env.OPENAI_BASE_URL
     process.env[getEnvVarForProvider(arg)] = '1'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/commands/provider.ts` around lines 89 - 91, The code currently deletes
user OpenAI environment variables (process.env.OPENAI_API_KEY and
process.env.OPENAI_BASE_URL) in provider selection; remove those delete calls
and stop mutating global process.env in provider.ts (specifically the lines
deleting CLAUDE_CODE_USE_OPENAI, OPENAI_API_KEY, OPENAI_BASE_URL). Instead,
implement provider-specific overrides using local variables or a scoped config
object (e.g., use a local const/openaiConfig or providerConfig passed into
functions that need credentials) so other tools relying on OPENAI_* are not
affected; if temporary overrides are required, set and restore them locally
rather than permanently deleting them.
🤖 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 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.
- Around line 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.
- Around line 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.
- Around line 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.

---

Duplicate comments:
In `@src/commands/provider.ts`:
- Around line 22-29: getMergedEnv currently declares a return type of
Record<string, string> but copies from process.env whose values are string |
undefined, which is unsound; change the function signature and internal typing
to return Record<string, string | undefined> (or alternatively coerce/filter
undefined values to strings if you want to guarantee no undefineds), update the
merged variable to reflect ProcessEnv typing, and ensure any callers of
getMergedEnv handle the string | undefined value; refer to getMergedEnv,
getSettings_DEPRECATED, merged and process.env when making the change.
- Around line 89-91: The code currently deletes user OpenAI environment
variables (process.env.OPENAI_API_KEY and process.env.OPENAI_BASE_URL) in
provider selection; remove those delete calls and stop mutating global
process.env in provider.ts (specifically the lines deleting
CLAUDE_CODE_USE_OPENAI, OPENAI_API_KEY, OPENAI_BASE_URL). Instead, implement
provider-specific overrides using local variables or a scoped config object
(e.g., use a local const/openaiConfig or providerConfig passed into functions
that need credentials) so other tools relying on OPENAI_* are not affected; if
temporary overrides are required, set and restore them locally rather than
permanently deleting them.
🪄 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: f0019815-ff1c-4f46-a857-c5d6766eacf5

📥 Commits

Reviewing files that changed from the base of the PR and between b5d1dbc and ec5dfed.

📒 Files selected for processing (1)
  • src/commands/provider.ts

Comment on lines +1 to +6
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'
Copy link
Copy Markdown

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.

Comment on lines +42 to +43
updateSettingsForSource('userSettings', { modelType: undefined })
// Also clear all provider-specific env vars to prevent conflicts
Copy link
Copy Markdown

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.

Comment on lines +62 to +71
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.`,
}
}
Copy link
Copy Markdown

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.

Comment on lines +77 to +86
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}.` }
Copy link
Copy Markdown

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.

@2228293026
Copy link
Copy Markdown
Contributor Author

神经claude code把项目代码全部格式化了一遍

@2228293026 2228293026 closed this Apr 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant