mirror of
https://github.com/Gitlawb/openclaude.git
synced 2026-11-13 07:53:41 -04:00
fix(gemini-vertex): address CodeRabbit review on the routing PR
- providerProfile.ts: honor an explicit shell GEMINI_VERTEX_AUTH_MODE over the
persisted/default value (same precedence as model/project).
- providerProfiles.ts: validate the active Vertex project in the profile
alignment check so a project drift forces re-apply instead of routing to the
wrong project (only when the profile pins a project; ADC-resolved stays free).
- messages.ts: harden isNotEmptyMessage against malformed user/assistant slots
({ type } / { message: {} }) that previously crashed on message.message.content;
add a regression test.
- providerConfig.local.test.ts: clear a possibly-leaked CLAUDE_CODE_USE_OPENAI so
the Gemini Vertex cache-scope assertion can't pass/fail for the wrong reason.
The Critical "typecheck must pass" item is a false positive: both
`bun run typecheck` and `bun run typecheck:type-tests` pass cleanly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
c97d9ff5fd
commit
e2e12c9201
@@ -330,11 +330,18 @@ test('does not classify Gemini Vertex sessions as first-party bootstrap traffic'
|
||||
// Maintainer P2: with only the Gemini Vertex flag set, the cache scope
|
||||
// must be null (third-party), not 'firstParty' — otherwise bootstrap
|
||||
// options are fetched through the Anthropic first-party path.
|
||||
const priorUseOpenAI = process.env.CLAUDE_CODE_USE_OPENAI
|
||||
// A leaked CLAUDE_CODE_USE_OPENAI from the runner env would take the OpenAI
|
||||
// branch and make this pass/fail for the wrong reason.
|
||||
delete process.env.CLAUDE_CODE_USE_OPENAI
|
||||
process.env.CLAUDE_CODE_USE_GEMINI_VERTEX = '1'
|
||||
try {
|
||||
expect(getAdditionalModelOptionsCacheScope()).toBeNull()
|
||||
} finally {
|
||||
delete process.env.CLAUDE_CODE_USE_GEMINI_VERTEX
|
||||
if (priorUseOpenAI !== undefined) {
|
||||
process.env.CLAUDE_CODE_USE_OPENAI = priorUseOpenAI
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
import { describe, expect, test } from 'bun:test'
|
||||
import { isNotEmptyMessage } from './messages.js'
|
||||
import type { Message } from '../types/message.js'
|
||||
|
||||
describe('isNotEmptyMessage malformed-history hardening', () => {
|
||||
test('treats falsy / non-object slots as empty', () => {
|
||||
expect(isNotEmptyMessage(undefined as unknown as Message)).toBe(false)
|
||||
expect(isNotEmptyMessage(null as unknown as Message)).toBe(false)
|
||||
expect(isNotEmptyMessage('oops' as unknown as Message)).toBe(false)
|
||||
})
|
||||
|
||||
test('does not crash on malformed user/assistant slots', () => {
|
||||
// These passed the object guard but have no message.content and used to
|
||||
// crash when it was read.
|
||||
expect(isNotEmptyMessage({ type: 'assistant' } as unknown as Message)).toBe(false)
|
||||
expect(
|
||||
isNotEmptyMessage({ type: 'user', message: {} } as unknown as Message),
|
||||
).toBe(false)
|
||||
expect(
|
||||
isNotEmptyMessage({ type: 'user', message: { content: null } } as unknown as Message),
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
test('still reports a non-empty string user message', () => {
|
||||
expect(
|
||||
isNotEmptyMessage({
|
||||
type: 'user',
|
||||
message: { content: 'hello' },
|
||||
} as unknown as Message),
|
||||
).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -718,6 +718,14 @@ export function isNotEmptyMessage(message: Message): boolean {
|
||||
return true
|
||||
}
|
||||
|
||||
// Defensive: user/assistant slots can be malformed (e.g. `{ type: 'user' }`
|
||||
// or `{ type: 'user', message: {} }`) after an error path; reading
|
||||
// message.message.content below would then crash. Treat them as empty.
|
||||
const inner = (message as { message?: { content?: unknown } }).message
|
||||
if (!inner || typeof inner !== 'object' || inner.content == null) {
|
||||
return false
|
||||
}
|
||||
|
||||
if (typeof message.message.content === 'string') {
|
||||
return message.message.content.trim().length > 0
|
||||
}
|
||||
|
||||
@@ -1613,12 +1613,15 @@ export async function buildLaunchEnv(options: {
|
||||
}
|
||||
|
||||
if (selectedProfile === 'gemini-vertex') {
|
||||
// Shell value wins over the persisted one (same precedence as model/project
|
||||
// below) so an explicit GEMINI_VERTEX_AUTH_MODE isn't silently replaced.
|
||||
const isVertexAuthMode = (value: string | undefined) =>
|
||||
value === 'access-token' || value === 'adc' ? value : undefined
|
||||
const env = buildGeminiVertexProfileEnv({
|
||||
authMode:
|
||||
persistedEnv.GEMINI_VERTEX_AUTH_MODE === 'access-token' ||
|
||||
persistedEnv.GEMINI_VERTEX_AUTH_MODE === 'adc'
|
||||
? persistedEnv.GEMINI_VERTEX_AUTH_MODE
|
||||
: 'adc',
|
||||
isVertexAuthMode(processEnv.GEMINI_VERTEX_AUTH_MODE) ??
|
||||
isVertexAuthMode(persistedEnv.GEMINI_VERTEX_AUTH_MODE) ??
|
||||
'adc',
|
||||
model: processEnv.GEMINI_VERTEX_MODEL || persistedEnv.GEMINI_VERTEX_MODEL,
|
||||
project:
|
||||
processEnv.GEMINI_VERTEX_PROJECT ||
|
||||
|
||||
@@ -624,6 +624,18 @@ function isProcessEnvAlignedWithProfile(
|
||||
}
|
||||
|
||||
if (compatibilityMode === 'gemini-vertex') {
|
||||
// The profile stores its Vertex project in baseUrl. When it pins one, treat
|
||||
// the env as aligned only if the active project still matches — otherwise a
|
||||
// project drift would skip re-apply and route to the wrong project. When the
|
||||
// profile has no explicit project (ADC resolves it), don't constrain it.
|
||||
const activeProject =
|
||||
trimOrUndefined(processEnv.GEMINI_VERTEX_PROJECT) ??
|
||||
trimOrUndefined(processEnv.GOOGLE_CLOUD_PROJECT) ??
|
||||
trimOrUndefined(processEnv.GCLOUD_PROJECT) ??
|
||||
trimOrUndefined(processEnv.GOOGLE_PROJECT_ID)
|
||||
const projectAligned = profile.baseUrl
|
||||
? sameOptionalEnvValue(activeProject, profile.baseUrl)
|
||||
: true
|
||||
return (
|
||||
processEnv.CLAUDE_CODE_USE_GEMINI_VERTEX !== undefined &&
|
||||
processEnv.CLAUDE_CODE_USE_OPENAI === undefined &&
|
||||
@@ -635,7 +647,8 @@ function isProcessEnvAlignedWithProfile(
|
||||
processEnv.CLAUDE_CODE_USE_FOUNDRY === undefined &&
|
||||
processEnv.OPENAI_BASE_URL === undefined &&
|
||||
processEnv.OPENAI_MODEL === undefined &&
|
||||
sameOptionalEnvValue(processEnv.GEMINI_VERTEX_MODEL, getPrimaryModel(profile.model))
|
||||
sameOptionalEnvValue(processEnv.GEMINI_VERTEX_MODEL, getPrimaryModel(profile.model)) &&
|
||||
projectAligned
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user