From 2566cd79067b8e1a11db6ba96bd8d3636a2dd4fd Mon Sep 17 00:00:00 2001 From: gitdevine <210788251+gitdevine@users.noreply.github.com> Date: Wed, 24 Jun 2026 18:56:31 +0200 Subject: [PATCH] refactor(provider-flags): derive flag-agnostic checks from the registry Address jatmn review 4558518184 on PR #1614: - Export hasAnyTruthyProviderSelectionFlag / hasConflictingProviderFlag from providerSelectionFlags.ts so the flag-agnostic call sites derive from the single source of truth instead of hand-enumerating CLAUDE_CODE_USE_* flags. Adopted by hasExplicitProviderSelection, hasNoExplicitNonOpenAICompatibleProvider, isUsing3PServices, and hasConflictingProviderFlagsForProfile (via an exhaustive activeProviderFlagForCompatibilityMode switch that preserves the github-enterprise -> CLAUDE_CODE_USE_GITHUB mapping and is compile-checked). Per-provider checks (hasConcreteProviderSelection, hasCompleteProviderSelection) stay as-is since they are not flag-agnostic. - Give the two source-scanning parity tests an explicit 30s timeout so they no longer flake under parallel test load. - Add unit tests for both helpers covering every registered flag. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/integrations/routeMetadata.ts | 12 +---- src/utils/auth.ts | 14 ++---- src/utils/providerProfile.ts | 12 +---- src/utils/providerProfiles.ts | 46 +++++++++++++---- src/utils/providerSelectionFlags.test.ts | 64 +++++++++++++++++++++++- src/utils/providerSelectionFlags.ts | 35 ++++++++++++- 6 files changed, 137 insertions(+), 46 deletions(-) diff --git a/src/integrations/routeMetadata.ts b/src/integrations/routeMetadata.ts index 165d63a3b..ec4c42c2d 100644 --- a/src/integrations/routeMetadata.ts +++ b/src/integrations/routeMetadata.ts @@ -14,6 +14,7 @@ import { } from './index.js' import { hasUsableOpenAICredential } from '../services/api/credentialPool.js' import { isEnvTruthy } from '../utils/envUtils.js' +import { hasAnyTruthyProviderSelectionFlag } from '../utils/providerSelectionFlags.js' import { PROVIDER_SELECTION_FLAGS } from '../utils/providerSelectionFlags.js' export type RouteDescriptor = GatewayDescriptor | VendorDescriptor @@ -468,16 +469,7 @@ function hasConflictingOpenAIBaseUrlForRoute( function hasNoExplicitNonOpenAICompatibleProvider( processEnv: NodeJS.ProcessEnv, ): boolean { - return ( - !isEnvTruthy(processEnv.CLAUDE_CODE_USE_OPENAI) && - !isEnvTruthy(processEnv.CLAUDE_CODE_USE_GITHUB) && - !isEnvTruthy(processEnv.CLAUDE_CODE_USE_GEMINI_VERTEX) && - !isEnvTruthy(processEnv.CLAUDE_CODE_USE_GEMINI) && - !isEnvTruthy(processEnv.CLAUDE_CODE_USE_MISTRAL) && - !isEnvTruthy(processEnv.CLAUDE_CODE_USE_BEDROCK) && - !isEnvTruthy(processEnv.CLAUDE_CODE_USE_VERTEX) && - !isEnvTruthy(processEnv.CLAUDE_CODE_USE_FOUNDRY) - ) + return !hasAnyTruthyProviderSelectionFlag(processEnv) } export function hasXaiEnvOnlyProviderIntent( diff --git a/src/utils/auth.ts b/src/utils/auth.ts index aba9d401d..50ed583db 100644 --- a/src/utils/auth.ts +++ b/src/utils/auth.ts @@ -54,6 +54,7 @@ import { isEnvTruthy, isRunningOnHomespace, } from './envUtils.js' +import { hasAnyTruthyProviderSelectionFlag } from './providerSelectionFlags.js' import { errorMessage } from './errors.js' import { execSyncWithDefaults_DEPRECATED } from './execFileNoThrow.js' import * as lockfile from './lockfile.js' @@ -1751,18 +1752,9 @@ export function getSubscriptionName(): string { } } -/** Check if using third-party services (Bedrock or Vertex or Foundry or OpenAI-compatible or Gemini or GitHub Models) */ +/** Check if any third-party provider-selection flag is active (derives from PROVIDER_SELECTION_FLAGS). */ export function isUsing3PServices(): boolean { - return !!( - isEnvTruthy(process.env.CLAUDE_CODE_USE_BEDROCK) || - isEnvTruthy(process.env.CLAUDE_CODE_USE_VERTEX) || - isEnvTruthy(process.env.CLAUDE_CODE_USE_FOUNDRY) || - isEnvTruthy(process.env.CLAUDE_CODE_USE_OPENAI) || - isEnvTruthy(process.env.CLAUDE_CODE_USE_GEMINI) || - isEnvTruthy(process.env.CLAUDE_CODE_USE_GEMINI_VERTEX) || - isEnvTruthy(process.env.CLAUDE_CODE_USE_MISTRAL) || - isEnvTruthy(process.env.CLAUDE_CODE_USE_GITHUB) - ) + return hasAnyTruthyProviderSelectionFlag() } /** diff --git a/src/utils/providerProfile.ts b/src/utils/providerProfile.ts index cbe5d9354..8681c157d 100644 --- a/src/utils/providerProfile.ts +++ b/src/utils/providerProfile.ts @@ -45,6 +45,7 @@ export { sanitizeProviderConfigValue, } from './providerSecrets.js' import { getClaudeConfigHomeDir, isEnvTruthy } from './envUtils.js' +import { hasAnyTruthyProviderSelectionFlag } from './providerSelectionFlags.js' export const PROFILE_FILE_NAME = '.openclaude-profile.json' export const DEFAULT_GEMINI_BASE_URL = @@ -1279,16 +1280,7 @@ export function hasExplicitProviderSelection( return true } - return ( - isEnvTruthy(processEnv.CLAUDE_CODE_USE_OPENAI) || - isEnvTruthy(processEnv.CLAUDE_CODE_USE_GITHUB) || - isEnvTruthy(processEnv.CLAUDE_CODE_USE_GEMINI) || - isEnvTruthy(processEnv.CLAUDE_CODE_USE_GEMINI_VERTEX) || - isEnvTruthy(processEnv.CLAUDE_CODE_USE_MISTRAL) || - isEnvTruthy(processEnv.CLAUDE_CODE_USE_BEDROCK) || - isEnvTruthy(processEnv.CLAUDE_CODE_USE_VERTEX) || - isEnvTruthy(processEnv.CLAUDE_CODE_USE_FOUNDRY) - ) + return hasAnyTruthyProviderSelectionFlag(processEnv) } function hasConcreteProviderSelection( diff --git a/src/utils/providerProfiles.ts b/src/utils/providerProfiles.ts index e69df5d96..0a6087bda 100644 --- a/src/utils/providerProfiles.ts +++ b/src/utils/providerProfiles.ts @@ -13,7 +13,11 @@ import { isEnvTruthy } from './envUtils.js' import { resolveGeminiVertexAuthMode } from './geminiAuth.js' import type { ModelOption } from './model/modelOptions.js' import { getPrimaryModel, parseModelList } from './providerModels.js' -import { PROVIDER_SELECTION_FLAGS } from './providerSelectionFlags.js' +import { + PROVIDER_SELECTION_FLAGS, + hasConflictingProviderFlag, + type ProviderSelectionFlag, +} from './providerSelectionFlags.js' import { buildCompatibilityProcessEnv, createProfileFile, @@ -547,19 +551,39 @@ function hasConflictingProviderFlagsForProfile( return hasProviderSelectionFlags(processEnv) } - return ( - (compatibilityMode !== 'openai' && processEnv.CLAUDE_CODE_USE_OPENAI !== undefined) || - (compatibilityMode !== 'gemini' && processEnv.CLAUDE_CODE_USE_GEMINI !== undefined) || - (compatibilityMode !== 'gemini-vertex' && processEnv.CLAUDE_CODE_USE_GEMINI_VERTEX !== undefined) || - (compatibilityMode !== 'mistral' && processEnv.CLAUDE_CODE_USE_MISTRAL !== undefined) || - (!isGithubCompatibilityMode(compatibilityMode) && - processEnv.CLAUDE_CODE_USE_GITHUB !== undefined) || - (compatibilityMode !== 'bedrock' && processEnv.CLAUDE_CODE_USE_BEDROCK !== undefined) || - (compatibilityMode !== 'vertex' && processEnv.CLAUDE_CODE_USE_VERTEX !== undefined) || - processEnv.CLAUDE_CODE_USE_FOUNDRY !== undefined + // The active profile's own flag is the only non-conflicting selection. The + // mapping is exhaustive (TS errors if a new compatibility mode is added), and + // ProfileCompatibilityMode never resolves to 'foundry', so the Foundry flag is + // always treated as a conflict — matching the prior hand-enumerated check. + return hasConflictingProviderFlag( + processEnv, + activeProviderFlagForCompatibilityMode(compatibilityMode), ) } +/** Maps a non-anthropic compatibility mode to the selection flag it owns. */ +function activeProviderFlagForCompatibilityMode( + compatibilityMode: Exclude, +): ProviderSelectionFlag { + switch (compatibilityMode) { + case 'openai': + return 'CLAUDE_CODE_USE_OPENAI' + case 'gemini': + return 'CLAUDE_CODE_USE_GEMINI' + case 'gemini-vertex': + return 'CLAUDE_CODE_USE_GEMINI_VERTEX' + case 'mistral': + return 'CLAUDE_CODE_USE_MISTRAL' + case 'github': + case 'github-enterprise': + return 'CLAUDE_CODE_USE_GITHUB' + case 'bedrock': + return 'CLAUDE_CODE_USE_BEDROCK' + case 'vertex': + return 'CLAUDE_CODE_USE_VERTEX' + } +} + /** * Whether the API client should route to the native Gemini Vertex client * based on the saved active profile alone (i.e. CLAUDE_CODE_USE_GEMINI_VERTEX diff --git a/src/utils/providerSelectionFlags.test.ts b/src/utils/providerSelectionFlags.test.ts index 6c53a98ba..02d4ede9b 100644 --- a/src/utils/providerSelectionFlags.test.ts +++ b/src/utils/providerSelectionFlags.test.ts @@ -1,7 +1,11 @@ import { describe, expect, test } from 'bun:test' import { readFileSync } from 'node:fs' import { join } from 'node:path' -import { PROVIDER_SELECTION_FLAGS } from './providerSelectionFlags.js' +import { + PROVIDER_SELECTION_FLAGS, + hasAnyTruthyProviderSelectionFlag, + hasConflictingProviderFlag, +} from './providerSelectionFlags.js' /** * Feature toggles that share the CLAUDE_CODE_USE_ prefix but do not select @@ -15,6 +19,9 @@ const NON_PROVIDER_USE_FLAGS = new Set([ ]) describe('PROVIDER_SELECTION_FLAGS parity', () => { + // These two tests synchronously walk the whole src/ tree, which is I/O-bound + // and can exceed Bun's 5000 ms default under parallel test load (~8.5 s cold, + // ~0.2 s isolated). A generous explicit timeout keeps them from flaking in CI. test('every CLAUDE_CODE_USE_* provider flag in src is registered', () => { const srcRoot = join(import.meta.dir, '..') const glob = new Bun.Glob('**/*.{ts,tsx}') @@ -42,7 +49,7 @@ describe('PROVIDER_SELECTION_FLAGS parity', () => { // startup-selection, env-propagation and cleanup paths all recognise it // (this is how gemini-vertex drifted out of the reference lists). expect(unregistered).toEqual([]) - }) + }, 30000) test('registered flags are actually consumed outside the registry definition', () => { const srcRoot = join(import.meta.dir, '..') @@ -63,5 +70,58 @@ describe('PROVIDER_SELECTION_FLAGS parity', () => { for (const flag of PROVIDER_SELECTION_FLAGS) { expect(allContent).toContain(flag) } + }, 30000) +}) + +describe('hasAnyTruthyProviderSelectionFlag', () => { + test('returns false for an empty env', () => { + expect(hasAnyTruthyProviderSelectionFlag({})).toBe(false) + }) + + // Every registered flag must be recognised by the shared helper — this is the + // guard that prevents a flag-agnostic call site (e.g. isUsing3PServices) from + // silently omitting a provider, as Gemini Vertex was before the registry. + test.each([...PROVIDER_SELECTION_FLAGS])( + 'recognises %s when truthy', + flag => { + expect(hasAnyTruthyProviderSelectionFlag({ [flag]: '1' })).toBe(true) + }, + ) + + test('ignores falsy values', () => { + expect( + hasAnyTruthyProviderSelectionFlag({ CLAUDE_CODE_USE_OPENAI: '0' }), + ).toBe(false) + }) +}) + +describe('hasConflictingProviderFlag', () => { + test('returns false for an empty env', () => { + expect(hasConflictingProviderFlag({})).toBe(false) + }) + + test('treats any present flag (even bare) as a conflict', () => { + expect(hasConflictingProviderFlag({ CLAUDE_CODE_USE_GEMINI: '' })).toBe(true) + }) + + test('does not flag the excepted active flag', () => { + expect( + hasConflictingProviderFlag( + { CLAUDE_CODE_USE_GEMINI_VERTEX: '1' }, + 'CLAUDE_CODE_USE_GEMINI_VERTEX', + ), + ).toBe(false) + }) + + test('flags a different provider even when the active flag is set', () => { + expect( + hasConflictingProviderFlag( + { + CLAUDE_CODE_USE_GEMINI_VERTEX: '1', + CLAUDE_CODE_USE_OPENAI: '1', + }, + 'CLAUDE_CODE_USE_GEMINI_VERTEX', + ), + ).toBe(true) }) }) diff --git a/src/utils/providerSelectionFlags.ts b/src/utils/providerSelectionFlags.ts index 51e2eeb6c..acdaa55ba 100644 --- a/src/utils/providerSelectionFlags.ts +++ b/src/utils/providerSelectionFlags.ts @@ -9,9 +9,12 @@ * registered here, which is how the Gemini Vertex provider drifted out of * several reference lists in the first place. * - * Dependency-free on purpose: both low-level (model/providers.ts) and - * high-level (providerProfiles.ts) modules import it without cycles. + * Leaf module on purpose: it only depends on envUtils (itself a leaf), so + * both low-level (model/providers.ts) and high-level (providerProfiles.ts) + * modules import it without cycles. */ +import { isEnvTruthy } from './envUtils.js' + export const PROVIDER_SELECTION_FLAGS = [ 'CLAUDE_CODE_USE_OPENAI', 'CLAUDE_CODE_USE_GEMINI', @@ -24,3 +27,31 @@ export const PROVIDER_SELECTION_FLAGS = [ ] as const export type ProviderSelectionFlag = (typeof PROVIDER_SELECTION_FLAGS)[number] + +/** + * True when any registered provider-selection flag is set to a truthy value. + * Derives from PROVIDER_SELECTION_FLAGS so flag-agnostic call sites (explicit + * selection, 3P-service detection, non-OpenAI-compatible gating) never drift + * out of sync — this is exactly the check that silently omitted Gemini Vertex. + */ +export function hasAnyTruthyProviderSelectionFlag( + processEnv: NodeJS.ProcessEnv = process.env, +): boolean { + return PROVIDER_SELECTION_FLAGS.some(flag => isEnvTruthy(processEnv[flag])) +} + +/** + * True when any registered provider-selection flag other than `exceptFlag` is + * present in env. Uses presence (`!== undefined`), not truthiness, because even + * a bare `CLAUDE_CODE_USE_=` signals intent for the conflict check. Pass the + * flag of the active profile to detect a conflicting selection for any other + * provider. + */ +export function hasConflictingProviderFlag( + processEnv: NodeJS.ProcessEnv = process.env, + exceptFlag?: ProviderSelectionFlag, +): boolean { + return PROVIDER_SELECTION_FLAGS.some( + flag => flag !== exceptFlag && processEnv[flag] !== undefined, + ) +}