Skip to content

fix(auth): honor settings-based providers - #1326

Open
ai-yang wants to merge 1 commit into
claude-code-best:mainfrom
ai-yang:agent/settings-provider-gates
Open

fix(auth): honor settings-based providers#1326
ai-yang wants to merge 1 commit into
claude-code-best:mainfrom
ai-yang:agent/settings-provider-gates

Conversation

@ai-yang

@ai-yang ai-yang commented Aug 1, 2026

Copy link
Copy Markdown

Fixes #1325

根因

Console availability、auth status 与 Anthropic auth gate 使用了不一致的第三方判定。env provider 已由 #1235 覆盖,但 settings.modelType,尤其 Grok 路径,仍可能落入 first-party 分支。

修复

  • 新增零依赖 isThirdPartyAPIProvider() 分类 helper。
  • Console-only 命令改为基于 getAPIProvider() 的完整分类。
  • claude auth status 对 settings/env provider 统一输出 third_party
  • isAnthropicAuthEnabled() 对已确定的第三方 provider 直接返回 false,避免无意义的 Anthropic credential 探测。
  • 保留 OPENAI_BASE_URL / GEMINI_BASE_URL 兼容判断。
  • 明确保留 isUsing3PServices() 的 env-only 语义。
  • /login/logout 继续使用原条件,不改变入口可见性。

兼容性

first-party、六种 CLAUDE_CODE_USE_* env provider、OpenAI/Gemini base URL 及 settings OpenAI/Gemini/Grok 均有对照测试。auth status 的既有 credential probing 与 JSON 字段保持不变。

测试

  • Bun 1.3.13 定向 wrapper 连续两次:每次 34 pass / 0 fail;隔离 gate 矩阵 38 pass / 0 fail
  • Biome 2.4.12
  • git diff --check
  • 上游 main@987e550 基线完整 CI
  • 修复提交完整 CI 连续两次(包含 Biome、typecheck、coverage、Vite build)

CI 证据:

Summary by CodeRabbit

  • Bug Fixes

    • Improved authentication and command availability detection across first-party and third-party API providers.
    • Ensured login, logout, and authentication status accurately reflect the selected provider and configured endpoints.
  • Tests

    • Added comprehensive coverage for provider classification, authentication behavior, command gates, and provider-specific settings.

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The change centralizes third-party provider classification and applies it to authentication detection, console command availability, auth-status reporting, and provider-gating regression tests.

Changes

Provider gate classification and integration

Layer / File(s) Summary
Centralized provider classification
src/utils/model/providerClassification.ts, src/utils/model/providers.ts, src/utils/model/__tests__/providers.test.ts
Adds the shared APIProvider type and isThirdPartyAPIProvider helper. Re-exports both from providers.ts. Adds provider classification tests.
Authentication and command gate integration
src/utils/auth.ts, src/commands.ts, src/cli/handlers/auth.ts
Uses getAPIProvider() for third-party detection in authentication status, Anthropic auth checks, and console command availability. Preserves environment-based compatibility checks and login/logout visibility.
Provider-gating regression coverage
src/utils/model/__tests__/providerGates.runner.ts, src/utils/model/__tests__/providerGates.test.ts
Adds isolated tests for settings-based and environment-based providers, authentication, command visibility, auth status, and login/logout behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: claude-code-best

Sequence Diagram(s)

sequenceDiagram
  participant ProviderSettings
  participant getAPIProvider
  participant isThirdPartyAPIProvider
  participant AuthAndCommands
  ProviderSettings->>getAPIProvider: provide configured API provider
  getAPIProvider->>isThirdPartyAPIProvider: classify provider
  isThirdPartyAPIProvider->>AuthAndCommands: return third-party status
  AuthAndCommands->>AuthAndCommands: apply authentication and command gates
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the primary change: honoring settings-based providers during authentication handling.
Linked Issues check ✅ Passed The changes satisfy issue [#1325] by applying unified provider classification and preserving the required compatibility and visibility behavior.
Out of Scope Changes check ✅ Passed All code and test changes directly support settings-based provider detection, authentication gates, and the linked issue requirements.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@ai-yang
ai-yang force-pushed the agent/settings-provider-gates branch from f360ef0 to b9f1cff Compare August 1, 2026 02:30
@ai-yang
ai-yang marked this pull request as ready for review August 1, 2026 04:10

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/utils/auth.ts (1)

138-147: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Update the stale rationale comment.

The comment at Line 139 lists "1. Using 3rd party services (Bedrock/Vertex/Foundry)" as a reason for shouldDisableAuth. This item no longer applies here: the is3P branch was removed from shouldDisableAuth, and third-party providers are now rejected by the earlier if (is3P) return false at Line 123. Update the comment so it does not attribute third-party gating to code that no longer performs it in this block.

📝 Proposed comment fix
-  // Disable Anthropic auth if:
-  // 1. Using 3rd party services (Bedrock/Vertex/Foundry)
-  // 2. User has an external API key (regardless of proxy configuration)
-  // 3. User has an external auth token (regardless of proxy configuration)
+  // Disable Anthropic auth if (third-party providers already handled above):
+  // 1. User has an external API key (regardless of proxy configuration)
+  // 2. User has an external auth token (regardless of proxy configuration)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/auth.ts` around lines 138 - 147, Update the rationale comment above
shouldDisableAuth to remove the outdated third-party services item and describe
only the external API key and external auth token conditions handled by this
expression; leave the existing shouldDisableAuth logic and earlier is3P handling
unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/utils/auth.ts`:
- Around line 138-147: Update the rationale comment above shouldDisableAuth to
remove the outdated third-party services item and describe only the external API
key and external auth token conditions handled by this expression; leave the
existing shouldDisableAuth logic and earlier is3P handling unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e57aa172-7c28-4796-8bb7-422d17a5f592

📥 Commits

Reviewing files that changed from the base of the PR and between 987e550 and b9f1cff.

📒 Files selected for processing (8)
  • src/cli/handlers/auth.ts
  • src/commands.ts
  • src/utils/auth.ts
  • src/utils/model/__tests__/providerGates.runner.ts
  • src/utils/model/__tests__/providerGates.test.ts
  • src/utils/model/__tests__/providers.test.ts
  • src/utils/model/providerClassification.ts
  • src/utils/model/providers.ts

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.

auth: settings.modelType 第三方 provider 被误判为 Anthropic first-party

1 participant