mirror of
https://github.com/Gitlawb/openclaude.git
synced 2026-11-06 12:50:54 -04:00
fix(gemini-vertex): tighten ADC completeness, neutral project placeholder, hermetic tests
Addresses CodeRabbit review on 6236de2 (5 findings):
- hasCompleteProviderSelection: use the *effective* auth mode
(resolveGeminiVertexAuthMode) so a GEMINI_ACCESS_TOKEN alongside a
GOOGLE_APPLICATION_CREDENTIALS file (which resolves to access-token and still
needs a project) is no longer treated as complete. The credentials file only
counts when the effective mode stays ADC. +2 regressions (token+creds, ADC).
- StartupScreen / buildCurrentProviderSummary: show a neutral
"<ADC-resolved project>" placeholder when an explicit ADC credential is
present (the client derives credential.projectId at runtime) instead of the
imperative "<set GEMINI_VERTEX_PROJECT>"; keep the actionable hint for
access-token/ambient setups. +startup regression.
- client.test.ts: add an ADC-only routing regression asserting the generated
Vertex URL uses the credential-discovered project (no env/profile project).
- Test isolation: snapshot a structuredClone of the global config before
mutating it (StartupScreen), and save/restore DISABLE_ERROR_REPORTING +
essential-traffic env so the logError gate control case is hermetic in CI.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
6236de2500
commit
365cc2e13b
@@ -318,12 +318,19 @@ export function buildCurrentProviderSummary(options?: {
|
||||
const vertexMetadata = getProviderPresetUiMetadata('gemini-vertex', vertexEnv)
|
||||
const location = getGeminiVertexLocation(vertexEnv)
|
||||
const project = getGeminiVertexProjectId(vertexEnv)
|
||||
// Mirror the startup screen: the native client always targets
|
||||
// /projects/<project>/locations/<location> and requires a project, so
|
||||
// surface a project-required state rather than a project-less endpoint.
|
||||
const endpoint = project
|
||||
? `https://aiplatform.googleapis.com/v1/projects/${project}/locations/${location}`
|
||||
: `https://aiplatform.googleapis.com/v1/projects/<set GEMINI_VERTEX_PROJECT>/locations/${location}`
|
||||
// Mirror the startup screen: an explicit ADC credential lets the native
|
||||
// client derive the project at runtime (credential.projectId), so show a
|
||||
// neutral placeholder there; access-token/ambient setups genuinely require
|
||||
// a project, so keep the actionable hint rather than a project-less endpoint.
|
||||
const adcWillResolveProject =
|
||||
vertexEnv.GEMINI_VERTEX_AUTH_MODE?.trim().toLowerCase() === 'adc' ||
|
||||
(vertexEnv.GOOGLE_APPLICATION_CREDENTIALS?.trim().length ?? 0) > 0
|
||||
const projectSegment =
|
||||
project ??
|
||||
(adcWillResolveProject
|
||||
? '<ADC-resolved project>'
|
||||
: '<set GEMINI_VERTEX_PROJECT>')
|
||||
const endpoint = `https://aiplatform.googleapis.com/v1/projects/${projectSegment}/locations/${location}`
|
||||
return {
|
||||
providerLabel: vertexMetadata.label,
|
||||
modelLabel: getSafeDisplayValue(
|
||||
|
||||
@@ -50,9 +50,11 @@ const ENV_KEYS = [
|
||||
'GEMINI_VERTEX_MODEL',
|
||||
'GEMINI_VERTEX_PROJECT',
|
||||
'GEMINI_VERTEX_LOCATION',
|
||||
'GEMINI_VERTEX_AUTH_MODE',
|
||||
'GOOGLE_CLOUD_PROJECT',
|
||||
'GCLOUD_PROJECT',
|
||||
'GOOGLE_PROJECT_ID',
|
||||
'GOOGLE_APPLICATION_CREDENTIALS',
|
||||
'MISTRAL_MODEL',
|
||||
'ANTHROPIC_MODEL',
|
||||
'CLAUDE_MODEL',
|
||||
@@ -383,6 +385,19 @@ describe('detectProvider — Gemini Vertex endpoint reflects the runtime project
|
||||
expect(result.baseUrl).toContain('GEMINI_VERTEX_PROJECT')
|
||||
})
|
||||
|
||||
test('shows a neutral placeholder for ADC setups that derive the project at runtime', () => {
|
||||
process.env.CLAUDE_CODE_USE_GEMINI_VERTEX = '1'
|
||||
// ADC credential present, no explicit project: getAnthropicClient resolves
|
||||
// the project from credential.projectId at runtime, so the display must not
|
||||
// imperatively demand GEMINI_VERTEX_PROJECT.
|
||||
process.env.GOOGLE_APPLICATION_CREDENTIALS = '/tmp/adc.json'
|
||||
|
||||
const result = detectProvider()
|
||||
expect(result.name).toBe('Gemini Vertex')
|
||||
expect(result.baseUrl).toContain('ADC-resolved project')
|
||||
expect(result.baseUrl).not.toContain('GEMINI_VERTEX_PROJECT')
|
||||
})
|
||||
|
||||
test('detects Vertex for isEnvTruthy flag values (yes/on), not just 1/true', () => {
|
||||
process.env.CLAUDE_CODE_USE_GEMINI_VERTEX = 'yes'
|
||||
process.env.GEMINI_VERTEX_PROJECT = 'my-project'
|
||||
@@ -409,7 +424,9 @@ describe('detectProvider — Gemini Vertex endpoint reflects the runtime project
|
||||
// active profile, exactly like getAnthropicClient. The profile's project
|
||||
// (stored in baseUrl) and model must drive the startup display instead of
|
||||
// falling through to the default Anthropic/OpenAI branch.
|
||||
const restore = getGlobalConfig()
|
||||
// Snapshot an immutable clone so the finally block fully resets the config
|
||||
// instead of restoring a reference that saveGlobalConfig may have mutated.
|
||||
const restore = structuredClone(getGlobalConfig())
|
||||
saveGlobalConfig(current => ({
|
||||
...current,
|
||||
providerProfiles: [
|
||||
|
||||
@@ -127,13 +127,21 @@ export function detectProvider(modelOverride?: string): { name: string; model: s
|
||||
DEFAULT_GEMINI_VERTEX_MODEL
|
||||
const location = getGeminiVertexLocation(process.env)
|
||||
const project = profileProject ?? getGeminiVertexProjectId(process.env)
|
||||
// The native client always targets /projects/<project>/locations/<location>
|
||||
// and throws when no project resolves. Mirror that contract here: when a
|
||||
// project is missing, surface a clear "project required" state instead of a
|
||||
// project-less endpoint that the runtime would never actually call.
|
||||
const baseUrl = project
|
||||
? `https://aiplatform.googleapis.com/v1/projects/${project}/locations/${location}`
|
||||
: `https://aiplatform.googleapis.com/v1/projects/<set GEMINI_VERTEX_PROJECT>/locations/${location}`
|
||||
// The native client always targets /projects/<project>/locations/<location>.
|
||||
// With an explicit ADC credential it can derive the project at runtime
|
||||
// (credential.projectId), so a missing project is not necessarily an error
|
||||
// there — show a neutral placeholder. For access-token/ambient setups the
|
||||
// client throws without a project, so keep the actionable "set the project"
|
||||
// hint instead of a project-less endpoint the runtime would never call.
|
||||
const adcWillResolveProject =
|
||||
process.env.GEMINI_VERTEX_AUTH_MODE?.trim().toLowerCase() === 'adc' ||
|
||||
(process.env.GOOGLE_APPLICATION_CREDENTIALS?.trim().length ?? 0) > 0
|
||||
const projectSegment =
|
||||
project ??
|
||||
(adcWillResolveProject
|
||||
? '<ADC-resolved project>'
|
||||
: '<set GEMINI_VERTEX_PROJECT>')
|
||||
const baseUrl = `https://aiplatform.googleapis.com/v1/projects/${projectSegment}/locations/${location}`
|
||||
return { name: 'Gemini Vertex', model, baseUrl, isLocal: false }
|
||||
}
|
||||
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
registerGateway,
|
||||
} from '../../integrations/index.js'
|
||||
import * as providerProfilesModule from '../../utils/providerProfiles.js'
|
||||
import * as geminiAuthModule from '../../utils/geminiAuth.js'
|
||||
import { getAnthropicClient } from './client.js'
|
||||
|
||||
type FetchType = typeof globalThis.fetch
|
||||
@@ -553,6 +554,64 @@ test('saved-profile-only Vertex prefers the profile project/model over ambient e
|
||||
}
|
||||
})
|
||||
|
||||
test('ADC-only Vertex routing uses the credential-discovered project', async () => {
|
||||
let capturedUrl: string | undefined
|
||||
|
||||
delete process.env.CLAUDE_CODE_USE_OPENAI
|
||||
delete process.env.CLAUDE_CODE_USE_GEMINI
|
||||
delete process.env.CLAUDE_CODE_USE_VERTEX
|
||||
process.env.CLAUDE_CODE_USE_GEMINI_VERTEX = '1'
|
||||
process.env.GEMINI_VERTEX_AUTH_MODE = 'adc'
|
||||
process.env.GEMINI_VERTEX_LOCATION = 'us-central1'
|
||||
process.env.GEMINI_VERTEX_MODEL = 'gemini-2.5-flash'
|
||||
// No GEMINI_VERTEX_PROJECT / alias and no token: the project must come from
|
||||
// the ADC credential (getAnthropicClient falls back to credential.projectId).
|
||||
|
||||
const spy = spyOn(
|
||||
geminiAuthModule,
|
||||
'resolveGeminiCredential',
|
||||
).mockResolvedValue({
|
||||
kind: 'adc',
|
||||
credential: 'adc-access-token',
|
||||
projectId: 'adc-discovered-project',
|
||||
} as Awaited<ReturnType<typeof geminiAuthModule.resolveGeminiCredential>>)
|
||||
|
||||
globalThis.fetch = (async (input, _init) => {
|
||||
capturedUrl =
|
||||
typeof input === 'string'
|
||||
? input
|
||||
: input instanceof URL
|
||||
? input.toString()
|
||||
: input.url
|
||||
return new Response(
|
||||
JSON.stringify({
|
||||
candidates: [{ content: { parts: [{ text: 'vertex ok' }] } }],
|
||||
}),
|
||||
{ headers: { 'Content-Type': 'application/json' } },
|
||||
)
|
||||
}) as FetchType
|
||||
|
||||
try {
|
||||
const client = (await getAnthropicClient({
|
||||
maxRetries: 0,
|
||||
model: 'gemini-2.5-flash',
|
||||
})) as unknown as ShimClient
|
||||
|
||||
await client.beta.messages.create({
|
||||
model: 'gemini-2.5-flash',
|
||||
messages: [{ role: 'user', content: 'hello' }],
|
||||
max_tokens: 64,
|
||||
stream: false,
|
||||
})
|
||||
|
||||
expect(capturedUrl).toBe(
|
||||
'https://us-central1-aiplatform.googleapis.com/v1/projects/adc-discovered-project/locations/us-central1/publishers/google/models/gemini-2.5-flash:generateContent',
|
||||
)
|
||||
} finally {
|
||||
spy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
test('Gemini Vertex provider uses GEMINI_VERTEX_MODEL instead of generic OPENAI_MODEL', async () => {
|
||||
let capturedUrl: string | undefined
|
||||
|
||||
|
||||
@@ -7,7 +7,10 @@ import { getInMemoryErrors, logError } from './log.js'
|
||||
// saved active profile (effective-provider check), so a saved-profile-only
|
||||
// Vertex session does not leak errors to the in-memory/reporting sink.
|
||||
describe('logError error-reporting gate — Gemini Vertex effective provider', () => {
|
||||
const PROVIDER_FLAGS = [
|
||||
// Provider-selection flags plus every other env input the logError() gate
|
||||
// reads (DISABLE_ERROR_REPORTING and the essential-traffic switch), so the
|
||||
// control case is hermetic and does not depend on leaked CI state.
|
||||
const GATING_ENV = [
|
||||
'CLAUDE_CODE_USE_GEMINI_VERTEX',
|
||||
'CLAUDE_CODE_USE_BEDROCK',
|
||||
'CLAUDE_CODE_USE_VERTEX',
|
||||
@@ -16,25 +19,28 @@ describe('logError error-reporting gate — Gemini Vertex effective provider', (
|
||||
'CLAUDE_CODE_USE_GEMINI',
|
||||
'CLAUDE_CODE_USE_MISTRAL',
|
||||
'CLAUDE_CODE_USE_GITHUB',
|
||||
'DISABLE_ERROR_REPORTING',
|
||||
'CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC',
|
||||
'DISABLE_TELEMETRY',
|
||||
]
|
||||
const savedFlags: Record<string, string | undefined> = {}
|
||||
const savedEnv: Record<string, string | undefined> = {}
|
||||
let restoreConfig: ReturnType<typeof getGlobalConfig>
|
||||
|
||||
beforeEach(() => {
|
||||
for (const flag of PROVIDER_FLAGS) {
|
||||
savedFlags[flag] = process.env[flag]
|
||||
delete process.env[flag]
|
||||
for (const key of GATING_ENV) {
|
||||
savedEnv[key] = process.env[key]
|
||||
delete process.env[key]
|
||||
}
|
||||
restoreConfig = getGlobalConfig()
|
||||
restoreConfig = structuredClone(getGlobalConfig())
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
saveGlobalConfig(() => restoreConfig)
|
||||
for (const flag of PROVIDER_FLAGS) {
|
||||
if (savedFlags[flag] === undefined) {
|
||||
delete process.env[flag]
|
||||
for (const key of GATING_ENV) {
|
||||
if (savedEnv[key] === undefined) {
|
||||
delete process.env[key]
|
||||
} else {
|
||||
process.env[flag] = savedFlags[flag]
|
||||
process.env[key] = savedEnv[key]
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
@@ -1507,6 +1507,68 @@ describe('applyActiveProviderProfileFromConfig', () => {
|
||||
}
|
||||
})
|
||||
|
||||
test('applies saved profile when a Vertex token coexists with a credentials file but no project', async () => {
|
||||
// A GEMINI_ACCESS_TOKEN makes the effective auth mode access-token even when
|
||||
// GOOGLE_APPLICATION_CREDENTIALS is also set, so the credentials file must
|
||||
// NOT make this token-only (no-project) selection look complete — the
|
||||
// native client would still throw "project is required".
|
||||
const { applyActiveProviderProfileFromConfig } =
|
||||
await importFreshProviderProfileModules()
|
||||
process.env.CLAUDE_CODE_USE_GEMINI_VERTEX = '1'
|
||||
process.env.GEMINI_ACCESS_TOKEN = 'ya29.startup-token'
|
||||
process.env.GOOGLE_APPLICATION_CREDENTIALS = '/tmp/adc.json'
|
||||
try {
|
||||
const applied = applyActiveProviderProfileFromConfig({
|
||||
providerProfiles: [
|
||||
buildProfile({
|
||||
id: 'saved_openai',
|
||||
baseUrl: 'https://api.openai.com/v1',
|
||||
model: 'gpt-4o',
|
||||
}),
|
||||
],
|
||||
activeProviderProfileId: 'saved_openai',
|
||||
} as any)
|
||||
|
||||
expect(applied?.id).toBe('saved_openai')
|
||||
expect(process.env.OPENAI_MODEL).toBe('gpt-4o')
|
||||
} finally {
|
||||
delete process.env.CLAUDE_CODE_USE_GEMINI_VERTEX
|
||||
delete process.env.GEMINI_ACCESS_TOKEN
|
||||
delete process.env.GOOGLE_APPLICATION_CREDENTIALS
|
||||
}
|
||||
})
|
||||
|
||||
test('keeps a Vertex ADC credentials-file startup selection (no project needed)', async () => {
|
||||
// A credentials file with the effective mode still ADC (no token) can derive
|
||||
// the project at runtime, so it is a complete selection and must not be
|
||||
// overridden by the saved profile.
|
||||
const { applyActiveProviderProfileFromConfig } =
|
||||
await importFreshProviderProfileModules()
|
||||
process.env.CLAUDE_CODE_USE_GEMINI_VERTEX = '1'
|
||||
process.env.GOOGLE_APPLICATION_CREDENTIALS = '/tmp/adc.json'
|
||||
try {
|
||||
const applied = applyActiveProviderProfileFromConfig({
|
||||
providerProfiles: [
|
||||
buildProfile({
|
||||
id: 'saved_openai',
|
||||
baseUrl: 'https://api.openai.com/v1',
|
||||
model: 'gpt-4o',
|
||||
}),
|
||||
],
|
||||
activeProviderProfileId: 'saved_openai',
|
||||
} as any)
|
||||
|
||||
expect(applied).toBeUndefined()
|
||||
expect(process.env.CLAUDE_CODE_USE_GEMINI_VERTEX).toBe('1')
|
||||
expect(process.env.GOOGLE_APPLICATION_CREDENTIALS).toBe('/tmp/adc.json')
|
||||
expect(process.env.CLAUDE_CODE_USE_OPENAI).toBeUndefined()
|
||||
expect(process.env.OPENAI_MODEL).toBeUndefined()
|
||||
} finally {
|
||||
delete process.env.CLAUDE_CODE_USE_GEMINI_VERTEX
|
||||
delete process.env.GOOGLE_APPLICATION_CREDENTIALS
|
||||
}
|
||||
})
|
||||
|
||||
test('keeps Gemini Vertex access-token startup selection when a project is set', async () => {
|
||||
// With a project alias the access-token selection is usable, so it stays
|
||||
// and the saved profile must not hijack it.
|
||||
|
||||
@@ -512,13 +512,16 @@ function hasCompleteProviderSelection(
|
||||
}
|
||||
// ADC is the only auth mode that can derive a project from the credentials
|
||||
// (getAnthropicClient falls back to credential.projectId for ADC only), so
|
||||
// an ADC selection is complete even without an explicit project alias.
|
||||
const authMode = trimOrUndefined(
|
||||
// an ADC selection is complete even without an explicit project alias. Use
|
||||
// the *effective* mode (resolveGeminiVertexAuthMode), not the raw env: a
|
||||
// GEMINI_ACCESS_TOKEN makes the runtime resolve access-token even when a
|
||||
// credentials file is also present, and that path still needs a project.
|
||||
const explicitAuthMode = trimOrUndefined(
|
||||
processEnv.GEMINI_VERTEX_AUTH_MODE,
|
||||
)?.toLowerCase()
|
||||
if (authMode === 'adc') return true
|
||||
if (explicitAuthMode === 'adc') return true
|
||||
if (
|
||||
authMode !== 'access-token' &&
|
||||
resolveGeminiVertexAuthMode(processEnv) === 'adc' &&
|
||||
trimOrUndefined(processEnv.GOOGLE_APPLICATION_CREDENTIALS) !== undefined
|
||||
) {
|
||||
return true
|
||||
|
||||
Reference in New Issue
Block a user