Merge branch 'main' into fix-subagent-tool-isolation

This commit is contained in:
AK
2026-03-11 13:02:45 -07:00
committed by GitHub
96 changed files with 3396 additions and 1536 deletions
@@ -10,6 +10,7 @@ import { OAuth2Client } from 'google-auth-library';
import {
UserTierId,
ActionStatus,
InitiationMethod,
type LoadCodeAssistResponse,
type GeminiUserTier,
type SetCodeAssistGlobalUserSettingRequest,
@@ -206,6 +207,7 @@ describe('CodeAssistServer', () => {
conversationOffered: expect.objectContaining({
traceId: 'test-trace-id',
status: ActionStatus.ACTION_STATUS_NO_ERROR,
initiationMethod: InitiationMethod.COMMAND,
streamingLatency: expect.objectContaining({
totalLatency: expect.stringMatching(/\d+s/),
firstMessageLatency: expect.stringMatching(/\d+s/),
@@ -274,6 +276,7 @@ describe('CodeAssistServer', () => {
expect.objectContaining({
conversationOffered: expect.objectContaining({
traceId: 'stream-trace-id',
initiationMethod: InitiationMethod.COMMAND,
}),
timestamp: expect.stringMatching(
/\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}.\d{3}Z/,
@@ -339,6 +339,7 @@ describe('telemetry', () => {
acceptedLines: '8',
removedLines: '3',
isAgentic: true,
initiationMethod: InitiationMethod.COMMAND,
}),
);
});
@@ -204,6 +204,7 @@ function createConversationInteraction(
removedLines,
language,
isAgentic: true,
initiationMethod: InitiationMethod.COMMAND,
};
}
+1
View File
@@ -330,6 +330,7 @@ export interface ConversationInteraction {
removedLines?: string;
language?: string;
isAgentic?: boolean;
initiationMethod?: InitiationMethod;
}
export interface FetchAdminControlsRequest {
+89
View File
@@ -492,6 +492,95 @@ describe('Server Config (config.ts)', () => {
expect(await config.getUserCaching()).toBeUndefined();
});
});
describe('getNumericalRoutingEnabled', () => {
it('should return true by default if there are no experiments', async () => {
const config = new Config(baseParams);
expect(await config.getNumericalRoutingEnabled()).toBe(true);
});
it('should return true if the remote flag is set to true', async () => {
const config = new Config({
...baseParams,
experiments: {
flags: {
[ExperimentFlags.ENABLE_NUMERICAL_ROUTING]: {
boolValue: true,
},
},
experimentIds: [],
},
} as unknown as ConfigParameters);
expect(await config.getNumericalRoutingEnabled()).toBe(true);
});
it('should return false if the remote flag is explicitly set to false', async () => {
const config = new Config({
...baseParams,
experiments: {
flags: {
[ExperimentFlags.ENABLE_NUMERICAL_ROUTING]: {
boolValue: false,
},
},
experimentIds: [],
},
} as unknown as ConfigParameters);
expect(await config.getNumericalRoutingEnabled()).toBe(false);
});
});
describe('getResolvedClassifierThreshold', () => {
it('should return 90 by default if there are no experiments', async () => {
const config = new Config(baseParams);
expect(await config.getResolvedClassifierThreshold()).toBe(90);
});
it('should return the remote flag value if it is within range (0-100)', async () => {
const config = new Config({
...baseParams,
experiments: {
flags: {
[ExperimentFlags.CLASSIFIER_THRESHOLD]: {
intValue: '75',
},
},
experimentIds: [],
},
} as unknown as ConfigParameters);
expect(await config.getResolvedClassifierThreshold()).toBe(75);
});
it('should return 90 if the remote flag is out of range (less than 0)', async () => {
const config = new Config({
...baseParams,
experiments: {
flags: {
[ExperimentFlags.CLASSIFIER_THRESHOLD]: {
intValue: '-10',
},
},
experimentIds: [],
},
} as unknown as ConfigParameters);
expect(await config.getResolvedClassifierThreshold()).toBe(90);
});
it('should return 90 if the remote flag is out of range (greater than 100)', async () => {
const config = new Config({
...baseParams,
experiments: {
flags: {
[ExperimentFlags.CLASSIFIER_THRESHOLD]: {
intValue: '110',
},
},
experimentIds: [],
},
} as unknown as ConfigParameters);
expect(await config.getResolvedClassifierThreshold()).toBe(90);
});
});
});
describe('refreshAuth', () => {
+35 -9
View File
@@ -783,7 +783,7 @@ export class Config implements McpContext, AgentLoopContext {
| undefined;
private disabledHooks: string[];
private experiments: Experiments | undefined;
private experimentsPromise: Promise<void> | undefined;
private experimentsPromise: Promise<Experiments | undefined> | undefined;
private hookSystem?: HookSystem;
private readonly onModelChange: ((model: string) => void) | undefined;
private readonly onReload:
@@ -1274,18 +1274,22 @@ export class Config implements McpContext, AgentLoopContext {
this.baseLlmClient = new BaseLlmClient(this.contentGenerator, this);
const codeAssistServer = getCodeAssistServer(this);
if (codeAssistServer?.projectId) {
await this.refreshUserQuota();
}
const quotaPromise = codeAssistServer?.projectId
? this.refreshUserQuota()
: Promise.resolve();
this.experimentsPromise = getExperiments(codeAssistServer)
.then((experiments) => {
this.setExperiments(experiments);
return experiments;
})
.catch((e) => {
debugLogger.error('Failed to fetch experiments', e);
return undefined;
});
await quotaPromise;
const authType = this.contentGeneratorConfig.authType;
if (
authType === AuthType.USE_GEMINI ||
@@ -1301,10 +1305,10 @@ export class Config implements McpContext, AgentLoopContext {
}
// Fetch admin controls
await this.ensureExperimentsLoaded();
const experiments = await this.experimentsPromise;
const adminControlsEnabled =
this.experiments?.flags[ExperimentFlags.ENABLE_ADMIN_CONTROLS]
?.boolValue ?? false;
experiments?.flags[ExperimentFlags.ENABLE_ADMIN_CONTROLS]?.boolValue ??
false;
const adminControls = await fetchAdminControls(
codeAssistServer,
this.getRemoteAdminSettings(),
@@ -2517,8 +2521,30 @@ export class Config implements McpContext, AgentLoopContext {
async getNumericalRoutingEnabled(): Promise<boolean> {
await this.ensureExperimentsLoaded();
return !!this.experiments?.flags[ExperimentFlags.ENABLE_NUMERICAL_ROUTING]
?.boolValue;
const flag =
this.experiments?.flags[ExperimentFlags.ENABLE_NUMERICAL_ROUTING];
return flag?.boolValue ?? true;
}
/**
* Returns the resolved complexity threshold for routing.
* If a remote threshold is provided and within range (0-100), it is returned.
* Otherwise, the default threshold (90) is returned.
*/
async getResolvedClassifierThreshold(): Promise<number> {
const remoteValue = await this.getClassifierThreshold();
const defaultValue = 90;
if (
remoteValue !== undefined &&
!isNaN(remoteValue) &&
remoteValue >= 0 &&
remoteValue <= 100
) {
return remoteValue;
}
return defaultValue;
}
async getClassifierThreshold(): Promise<number | undefined> {
+29 -7
View File
@@ -18,9 +18,16 @@ import { handleFallback } from '../fallback/handler.js';
import { getResponseText } from '../utils/partUtils.js';
import { reportError } from '../utils/errorReporting.js';
import { getErrorMessage } from '../utils/errors.js';
import { logMalformedJsonResponse } from '../telemetry/loggers.js';
import { MalformedJsonResponseEvent, LlmRole } from '../telemetry/types.js';
import { retryWithBackoff } from '../utils/retry.js';
import {
logMalformedJsonResponse,
logNetworkRetryAttempt,
} from '../telemetry/loggers.js';
import {
MalformedJsonResponseEvent,
LlmRole,
NetworkRetryAttemptEvent,
} from '../telemetry/types.js';
import { retryWithBackoff, getRetryErrorType } from '../utils/retry.js';
import { coreEvents } from '../utils/events.js';
import { getDisplayString } from '../config/models.js';
import type { ModelConfigKey } from '../services/modelConfigService.js';
@@ -331,14 +338,29 @@ export class BaseLlmClient {
this.authType ?? this.config.getContentGeneratorConfig()?.authType,
retryFetchErrors: this.config.getRetryFetchErrors(),
onRetry: (attempt, error, delayMs) => {
const actualMaxAttempts =
availabilityMaxAttempts ?? maxAttempts ?? DEFAULT_MAX_ATTEMPTS;
const modelName = getDisplayString(currentModel);
const errorType = getRetryErrorType(error);
coreEvents.emitRetryAttempt({
attempt,
maxAttempts:
availabilityMaxAttempts ?? maxAttempts ?? DEFAULT_MAX_ATTEMPTS,
maxAttempts: actualMaxAttempts,
delayMs,
error: error instanceof Error ? error.message : String(error),
model: getDisplayString(currentModel),
error: errorType,
model: modelName,
});
logNetworkRetryAttempt(
this.config,
new NetworkRetryAttemptEvent(
attempt,
actualMaxAttempts,
errorType,
delayMs,
modelName,
),
);
},
});
} catch (error) {
+7 -1
View File
@@ -85,14 +85,20 @@ vi.mock('../fallback/handler.js', () => ({
handleFallback: mockHandleFallback,
}));
const { mockLogContentRetry, mockLogContentRetryFailure } = vi.hoisted(() => ({
const {
mockLogContentRetry,
mockLogContentRetryFailure,
mockLogNetworkRetryAttempt,
} = vi.hoisted(() => ({
mockLogContentRetry: vi.fn(),
mockLogContentRetryFailure: vi.fn(),
mockLogNetworkRetryAttempt: vi.fn(),
}));
vi.mock('../telemetry/loggers.js', () => ({
logContentRetry: mockLogContentRetry,
logContentRetryFailure: mockLogContentRetryFailure,
logNetworkRetryAttempt: mockLogNetworkRetryAttempt,
}));
vi.mock('../telemetry/uiTelemetry.js', () => ({
+30 -7
View File
@@ -19,7 +19,11 @@ import {
type GenerateContentParameters,
} from '@google/genai';
import { toParts } from '../code_assist/converter.js';
import { retryWithBackoff, isRetryableError } from '../utils/retry.js';
import {
retryWithBackoff,
isRetryableError,
getRetryErrorType,
} from '../utils/retry.js';
import type { ValidationRequiredError } from '../utils/googleQuotaErrors.js';
import type { Config } from '../config/config.js';
import {
@@ -33,6 +37,7 @@ import type { CompletedToolCall } from './coreToolScheduler.js';
import {
logContentRetry,
logContentRetryFailure,
logNetworkRetryAttempt,
} from '../telemetry/loggers.js';
import {
ChatRecordingService,
@@ -41,6 +46,7 @@ import {
import {
ContentRetryEvent,
ContentRetryFailureEvent,
NetworkRetryAttemptEvent,
type LlmRole,
} from '../telemetry/types.js';
import { handleFallback } from '../fallback/handler.js';
@@ -412,7 +418,9 @@ export class GeminiChat {
}
const isContentError = error instanceof InvalidStreamError;
const errorType = isContentError ? error.type : 'NETWORK_ERROR';
const errorType = isContentError
? error.type
: getRetryErrorType(error);
if (
(isContentError && isGemini2Model(model)) ||
@@ -422,15 +430,28 @@ export class GeminiChat {
if (attempt < maxAttempts - 1) {
const delayMs = INVALID_CONTENT_RETRY_OPTIONS.initialDelayMs;
logContentRetry(
this.config,
new ContentRetryEvent(attempt, errorType, delayMs, model),
);
if (isContentError) {
logContentRetry(
this.config,
new ContentRetryEvent(attempt, errorType, delayMs, model),
);
} else {
logNetworkRetryAttempt(
this.config,
new NetworkRetryAttemptEvent(
attempt + 1,
maxAttempts,
errorType,
delayMs * (attempt + 1),
model,
),
);
}
coreEvents.emitRetryAttempt({
attempt: attempt + 1,
maxAttempts,
delayMs: delayMs * (attempt + 1),
error: error instanceof Error ? error.message : String(error),
error: errorType,
model,
});
await new Promise((res) =>
@@ -986,6 +1007,8 @@ export class GeminiChat {
status: call.status,
timestamp: new Date().toISOString(),
resultDisplay,
description:
'invocation' in call ? call.invocation?.getDescription() : undefined,
};
});
@@ -47,14 +47,20 @@ vi.mock('../utils/retry.js', async (importOriginal) => {
});
// Mock loggers
const { mockLogContentRetry, mockLogContentRetryFailure } = vi.hoisted(() => ({
const {
mockLogContentRetry,
mockLogContentRetryFailure,
mockLogNetworkRetryAttempt,
} = vi.hoisted(() => ({
mockLogContentRetry: vi.fn(),
mockLogContentRetryFailure: vi.fn(),
mockLogNetworkRetryAttempt: vi.fn(),
}));
vi.mock('../telemetry/loggers.js', () => ({
logContentRetry: mockLogContentRetry,
logContentRetryFailure: mockLogContentRetryFailure,
logNetworkRetryAttempt: mockLogNetworkRetryAttempt,
}));
describe('GeminiChat Network Retries', () => {
@@ -185,10 +191,10 @@ describe('GeminiChat Network Retries', () => {
expect(successChunk).toBeDefined();
// Verify retry logging
expect(mockLogContentRetry).toHaveBeenCalledWith(
expect(mockLogNetworkRetryAttempt).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
error_type: 'NETWORK_ERROR',
error_type: 'SERVER_ERROR',
}),
);
});
@@ -474,11 +480,11 @@ describe('GeminiChat Network Retries', () => {
);
expect(successChunk).toBeDefined();
// Verify retry logging was called with NETWORK_ERROR type
expect(mockLogContentRetry).toHaveBeenCalledWith(
// Verify retry logging was called with network error type
expect(mockLogNetworkRetryAttempt).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
error_type: 'NETWORK_ERROR',
error_type: 'ERR_SSL_SSLV3_ALERT_BAD_RECORD_MAC',
}),
);
});
File diff suppressed because it is too large Load Diff
+115 -63
View File
@@ -39,6 +39,26 @@ const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);
export const DEFAULT_CORE_POLICIES_DIR = path.join(__dirname, 'policies');
// Cache to prevent duplicate warnings in the same process
const emittedWarnings = new Set<string>();
/**
* Emits a warning feedback event only once per process.
*/
function emitWarningOnce(message: string): void {
if (!emittedWarnings.has(message)) {
coreEvents.emitFeedback('warning', message);
emittedWarnings.add(message);
}
}
/**
* Clears the emitted warnings cache. Used primarily for tests.
*/
export function clearEmittedPolicyWarnings(): void {
emittedWarnings.clear();
}
// Policy tier constants for priority calculation
export const DEFAULT_POLICY_TIER = 1;
export const EXTENSION_POLICY_TIER = 2;
@@ -89,33 +109,29 @@ export function getAlwaysAllowPriorityFraction(): number {
* @param policyPaths Optional user-provided policy paths (from --policy flag).
* When provided, these replace the default user policies directory.
* @param workspacePoliciesDir Optional path to a directory containing workspace policies.
* @param adminPolicyPaths Optional admin-provided policy paths (from --admin-policy flag).
* When provided, these supplement the default system policies directory.
*/
export function getPolicyDirectories(
defaultPoliciesDir?: string,
policyPaths?: string[],
workspacePoliciesDir?: string,
adminPolicyPaths?: string[],
): string[] {
const dirs = [];
return [
// Admin tier (highest priority)
Storage.getSystemPoliciesDir(),
...(adminPolicyPaths ?? []),
// Admin tier (highest priority)
dirs.push(Storage.getSystemPoliciesDir());
// User tier (second highest priority)
...(policyPaths ?? [Storage.getUserPoliciesDir()]),
// User tier (second highest priority)
if (policyPaths && policyPaths.length > 0) {
dirs.push(...policyPaths);
} else {
dirs.push(Storage.getUserPoliciesDir());
}
// Workspace Tier (third highest)
workspacePoliciesDir,
// Workspace Tier (third highest)
if (workspacePoliciesDir) {
dirs.push(workspacePoliciesDir);
}
// Default tier (lowest priority)
dirs.push(defaultPoliciesDir ?? DEFAULT_CORE_POLICIES_DIR);
return dirs;
// Default tier (lowest priority)
defaultPoliciesDir ?? DEFAULT_CORE_POLICIES_DIR,
].filter((dir): dir is string => !!dir);
}
/**
@@ -124,37 +140,40 @@ export function getPolicyDirectories(
*/
export function getPolicyTier(
dir: string,
defaultPoliciesDir?: string,
workspacePoliciesDir?: string,
context: {
defaultPoliciesDir?: string;
workspacePoliciesDir?: string;
adminPolicyPaths?: Set<string>;
systemPoliciesDir: string;
userPoliciesDir: string;
},
): number {
const USER_POLICIES_DIR = Storage.getUserPoliciesDir();
const ADMIN_POLICIES_DIR = Storage.getSystemPoliciesDir();
const normalizedDir = path.resolve(dir);
const normalizedUser = path.resolve(USER_POLICIES_DIR);
const normalizedAdmin = path.resolve(ADMIN_POLICIES_DIR);
if (normalizedDir === context.systemPoliciesDir) {
return ADMIN_POLICY_TIER;
}
if (context.adminPolicyPaths?.has(normalizedDir)) {
return ADMIN_POLICY_TIER;
}
if (normalizedDir === context.userPoliciesDir) {
return USER_POLICY_TIER;
}
if (
defaultPoliciesDir &&
normalizedDir === path.resolve(defaultPoliciesDir)
context.workspacePoliciesDir &&
normalizedDir === path.resolve(context.workspacePoliciesDir)
) {
return WORKSPACE_POLICY_TIER;
}
if (
context.defaultPoliciesDir &&
normalizedDir === path.resolve(context.defaultPoliciesDir)
) {
return DEFAULT_POLICY_TIER;
}
if (normalizedDir === path.resolve(DEFAULT_CORE_POLICIES_DIR)) {
return DEFAULT_POLICY_TIER;
}
if (normalizedDir === normalizedUser) {
return USER_POLICY_TIER;
}
if (
workspacePoliciesDir &&
normalizedDir === path.resolve(workspacePoliciesDir)
) {
return WORKSPACE_POLICY_TIER;
}
if (normalizedDir === normalizedAdmin) {
return ADMIN_POLICY_TIER;
}
return DEFAULT_POLICY_TIER;
}
@@ -178,21 +197,24 @@ export function formatPolicyError(error: PolicyFileError): string {
/**
* Filters out insecure policy directories (specifically the system policy directory).
* Supplemental admin policy paths are NOT subject to strict security checks as they
* are explicitly provided by the user/administrator via flags or settings.
* Emits warnings if insecure directories are found.
*/
async function filterSecurePolicyDirectories(
dirs: string[],
systemPoliciesDir: string,
): Promise<string[]> {
const systemPoliciesDir = path.resolve(Storage.getSystemPoliciesDir());
const results = await Promise.all(
dirs.map(async (dir) => {
// Only check security for system policies
if (path.resolve(dir) === systemPoliciesDir) {
const normalizedDir = path.resolve(dir);
const isSystemPolicy = normalizedDir === systemPoliciesDir;
if (isSystemPolicy) {
const { secure, reason } = await isDirectorySecure(dir);
if (!secure) {
const msg = `Security Warning: Skipping system policies from ${dir}: ${reason}`;
coreEvents.emitFeedback('warning', msg);
emitWarningOnce(msg);
return null;
}
}
@@ -271,40 +293,70 @@ export async function createPolicyEngineConfig(
approvalMode: ApprovalMode,
defaultPoliciesDir?: string,
): Promise<PolicyEngineConfig> {
const systemPoliciesDir = path.resolve(Storage.getSystemPoliciesDir());
const userPoliciesDir = path.resolve(Storage.getUserPoliciesDir());
let adminPolicyPaths = settings.adminPolicyPaths;
// Security: Ignore supplemental admin policies if the system directory already contains policies.
// This prevents flag-based overrides when a central system policy is established.
if (adminPolicyPaths?.length) {
try {
const files = await fs.readdir(systemPoliciesDir);
if (files.some((f) => f.endsWith('.toml'))) {
const msg = `Security Warning: Ignoring --admin-policy because system policies are already defined in ${systemPoliciesDir}`;
emitWarningOnce(msg);
adminPolicyPaths = undefined;
}
} catch (e) {
if (!isNodeError(e) || e.code !== 'ENOENT') {
debugLogger.warn(
`Failed to check system policies in ${systemPoliciesDir}`,
e,
);
}
}
}
const policyDirs = getPolicyDirectories(
defaultPoliciesDir,
settings.policyPaths,
settings.workspacePoliciesDir,
adminPolicyPaths,
);
const securePolicyDirs = await filterSecurePolicyDirectories(policyDirs);
const normalizedAdminPoliciesDir = path.resolve(
Storage.getSystemPoliciesDir(),
const adminPolicyPathsSet = adminPolicyPaths
? new Set(adminPolicyPaths.map((p) => path.resolve(p)))
: undefined;
const securePolicyDirs = await filterSecurePolicyDirectories(
policyDirs,
systemPoliciesDir,
);
const tierContext = {
defaultPoliciesDir,
workspacePoliciesDir: settings.workspacePoliciesDir,
adminPolicyPaths: adminPolicyPathsSet,
systemPoliciesDir,
userPoliciesDir,
};
const userProvidedPaths = settings.policyPaths
? new Set(settings.policyPaths.map((p) => path.resolve(p)))
: new Set<string>();
// Load policies from TOML files
const {
rules: tomlRules,
checkers: tomlCheckers,
errors,
} = await loadPoliciesFromToml(securePolicyDirs, (p) => {
const tier = getPolicyTier(
p,
defaultPoliciesDir,
settings.workspacePoliciesDir,
);
const normalizedPath = path.resolve(p);
const tier = getPolicyTier(normalizedPath, tierContext);
// If it's a user-provided path that isn't already categorized as ADMIN,
// treat it as USER tier.
if (
settings.policyPaths?.some(
(userPath) => path.resolve(userPath) === path.resolve(p),
)
) {
const normalizedPath = path.resolve(p);
if (normalizedPath !== normalizedAdminPoliciesDir) {
return USER_POLICY_TIER;
}
// If it's a user-provided path that isn't already categorized as ADMIN, treat it as USER tier.
if (userProvidedPaths.has(normalizedPath) && tier !== ADMIN_POLICY_TIER) {
return USER_POLICY_TIER;
}
return tier;
+2
View File
@@ -311,6 +311,8 @@ export interface PolicySettings {
mcpServers?: Record<string, { trust?: boolean }>;
// User provided policies that will replace the USER level policies in ~/.gemini/policies
policyPaths?: string[];
// Admin provided policies that will supplement the ADMIN level policies
adminPolicyPaths?: string[];
workspacePoliciesDir?: string;
}
+4 -3
View File
@@ -97,10 +97,10 @@ export function buildArgsPatterns(
* @returns A regex string that matches "file_path":"<path>" in a JSON string.
*/
export function buildFilePathArgsPattern(filePath: string): string {
// JSON.stringify safely encodes the path (handling quotes, backslashes, etc)
// and wraps it in double quotes. We simply prepend the key name and escape
// the entire sequence for Regex matching without any slicing.
const encodedPath = JSON.stringify(filePath);
// We must wrap the JSON string in escapeRegex to ensure regex control characters
// (like '.' in file extensions) are treated as literals, preventing overly broad
// matches (e.g. 'foo.ts' matching 'fooXts').
return escapeRegex(`"file_path":${encodedPath}`);
}
@@ -113,5 +113,6 @@ export function buildFilePathArgsPattern(filePath: string): string {
*/
export function buildPatternArgsPattern(pattern: string): string {
const encodedPattern = JSON.stringify(pattern);
// We use escapeRegex to ensure regex control characters are treated as literals.
return escapeRegex(`"pattern":${encodedPattern}`);
}
@@ -54,7 +54,10 @@ describe('ModelRouterService', () => {
vi.spyOn(mockConfig, 'getLocalLiteRtLmClient').mockReturnValue(
mockLocalLiteRtLmClient,
);
vi.spyOn(mockConfig, 'getNumericalRoutingEnabled').mockResolvedValue(false);
vi.spyOn(mockConfig, 'getNumericalRoutingEnabled').mockResolvedValue(true);
vi.spyOn(mockConfig, 'getResolvedClassifierThreshold').mockResolvedValue(
90,
);
vi.spyOn(mockConfig, 'getClassifierThreshold').mockResolvedValue(undefined);
vi.spyOn(mockConfig, 'getGemmaModelRouterSettings').mockReturnValue({
enabled: false,
@@ -182,8 +185,8 @@ describe('ModelRouterService', () => {
false,
undefined,
ApprovalMode.DEFAULT,
false,
undefined,
true,
'90',
);
expect(logModelRouting).toHaveBeenCalledWith(
mockConfig,
@@ -209,8 +212,8 @@ describe('ModelRouterService', () => {
true,
'Strategy failed',
ApprovalMode.DEFAULT,
false,
undefined,
true,
'90',
);
expect(logModelRouting).toHaveBeenCalledWith(
mockConfig,
@@ -78,10 +78,9 @@ export class ModelRouterService {
const [enableNumericalRouting, thresholdValue] = await Promise.all([
this.config.getNumericalRoutingEnabled(),
this.config.getClassifierThreshold(),
this.config.getResolvedClassifierThreshold(),
]);
const classifierThreshold =
thresholdValue !== undefined ? String(thresholdValue) : undefined;
const classifierThreshold = String(thresholdValue);
let failed = false;
let error_message: string | undefined;
@@ -56,6 +56,7 @@ describe('NumericalClassifierStrategy', () => {
getModel: vi.fn().mockReturnValue(PREVIEW_GEMINI_MODEL_AUTO),
getSessionId: vi.fn().mockReturnValue('control-group-id'), // Default to Control Group (Hash 71 >= 50)
getNumericalRoutingEnabled: vi.fn().mockResolvedValue(true),
getResolvedClassifierThreshold: vi.fn().mockResolvedValue(90),
getClassifierThreshold: vi.fn().mockResolvedValue(undefined),
getGemini31Launched: vi.fn().mockResolvedValue(false),
getUseCustomToolModel: vi.fn().mockImplementation(async () => {
@@ -152,12 +153,11 @@ describe('NumericalClassifierStrategy', () => {
expect(textPart?.text).toBe('simple task');
});
describe('A/B Testing Logic (Deterministic)', () => {
it('Control Group (SessionID "control-group-id" -> Threshold 50): Score 40 -> FLASH', async () => {
vi.mocked(mockConfig.getSessionId).mockReturnValue('control-group-id'); // Hash 71 -> Control
describe('Default Logic', () => {
it('should route to FLASH when score is below 90', async () => {
const mockApiResponse = {
complexity_reasoning: 'Standard task',
complexity_score: 40,
complexity_score: 80,
};
vi.mocked(mockBaseLlmClient.generateJson).mockResolvedValue(
mockApiResponse,
@@ -173,72 +173,17 @@ describe('NumericalClassifierStrategy', () => {
expect(decision).toEqual({
model: PREVIEW_GEMINI_FLASH_MODEL,
metadata: {
source: 'NumericalClassifier (Control)',
source: 'NumericalClassifier (Default)',
latencyMs: expect.any(Number),
reasoning: expect.stringContaining('Score: 40 / Threshold: 50'),
reasoning: expect.stringContaining('Score: 80 / Threshold: 90'),
},
});
});
it('Control Group (SessionID "control-group-id" -> Threshold 50): Score 60 -> PRO', async () => {
vi.mocked(mockConfig.getSessionId).mockReturnValue('control-group-id');
const mockApiResponse = {
complexity_reasoning: 'Complex task',
complexity_score: 60,
};
vi.mocked(mockBaseLlmClient.generateJson).mockResolvedValue(
mockApiResponse,
);
const decision = await strategy.route(
mockContext,
mockConfig,
mockBaseLlmClient,
mockLocalLiteRtLmClient,
);
expect(decision).toEqual({
model: PREVIEW_GEMINI_MODEL,
metadata: {
source: 'NumericalClassifier (Control)',
latencyMs: expect.any(Number),
reasoning: expect.stringContaining('Score: 60 / Threshold: 50'),
},
});
});
it('Strict Group (SessionID "test-session-1" -> Threshold 80): Score 60 -> FLASH', async () => {
vi.mocked(mockConfig.getSessionId).mockReturnValue('test-session-1'); // FNV Normalized 18 < 50 -> Strict
const mockApiResponse = {
complexity_reasoning: 'Complex task',
complexity_score: 60,
};
vi.mocked(mockBaseLlmClient.generateJson).mockResolvedValue(
mockApiResponse,
);
const decision = await strategy.route(
mockContext,
mockConfig,
mockBaseLlmClient,
mockLocalLiteRtLmClient,
);
expect(decision).toEqual({
model: PREVIEW_GEMINI_FLASH_MODEL, // Routed to Flash because 60 < 80
metadata: {
source: 'NumericalClassifier (Strict)',
latencyMs: expect.any(Number),
reasoning: expect.stringContaining('Score: 60 / Threshold: 80'),
},
});
});
it('Strict Group (SessionID "test-session-1" -> Threshold 80): Score 90 -> PRO', async () => {
vi.mocked(mockConfig.getSessionId).mockReturnValue('test-session-1');
it('should route to PRO when score is 90 or above', async () => {
const mockApiResponse = {
complexity_reasoning: 'Extreme task',
complexity_score: 90,
complexity_score: 95,
};
vi.mocked(mockBaseLlmClient.generateJson).mockResolvedValue(
mockApiResponse,
@@ -254,9 +199,9 @@ describe('NumericalClassifierStrategy', () => {
expect(decision).toEqual({
model: PREVIEW_GEMINI_MODEL,
metadata: {
source: 'NumericalClassifier (Strict)',
source: 'NumericalClassifier (Default)',
latencyMs: expect.any(Number),
reasoning: expect.stringContaining('Score: 90 / Threshold: 80'),
reasoning: expect.stringContaining('Score: 95 / Threshold: 90'),
},
});
});
@@ -265,6 +210,9 @@ describe('NumericalClassifierStrategy', () => {
describe('Remote Threshold Logic', () => {
it('should use the remote CLASSIFIER_THRESHOLD if provided (int value)', async () => {
vi.mocked(mockConfig.getClassifierThreshold).mockResolvedValue(70);
vi.mocked(mockConfig.getResolvedClassifierThreshold).mockResolvedValue(
70,
);
const mockApiResponse = {
complexity_reasoning: 'Test task',
complexity_score: 60,
@@ -292,6 +240,9 @@ describe('NumericalClassifierStrategy', () => {
it('should use the remote CLASSIFIER_THRESHOLD if provided (float value)', async () => {
vi.mocked(mockConfig.getClassifierThreshold).mockResolvedValue(45.5);
vi.mocked(mockConfig.getResolvedClassifierThreshold).mockResolvedValue(
45.5,
);
const mockApiResponse = {
complexity_reasoning: 'Test task',
complexity_score: 40,
@@ -319,6 +270,9 @@ describe('NumericalClassifierStrategy', () => {
it('should use PRO model if score >= remote CLASSIFIER_THRESHOLD', async () => {
vi.mocked(mockConfig.getClassifierThreshold).mockResolvedValue(30);
vi.mocked(mockConfig.getResolvedClassifierThreshold).mockResolvedValue(
30,
);
const mockApiResponse = {
complexity_reasoning: 'Test task',
complexity_score: 35,
@@ -344,13 +298,12 @@ describe('NumericalClassifierStrategy', () => {
});
});
it('should fall back to A/B testing if CLASSIFIER_THRESHOLD is not present in experiments', async () => {
it('should fall back to default logic if CLASSIFIER_THRESHOLD is not present in experiments', async () => {
// Mock getClassifierThreshold to return undefined
vi.mocked(mockConfig.getClassifierThreshold).mockResolvedValue(undefined);
vi.mocked(mockConfig.getSessionId).mockReturnValue('control-group-id'); // Should resolve to Control (50)
const mockApiResponse = {
complexity_reasoning: 'Test task',
complexity_score: 40,
complexity_score: 80,
};
vi.mocked(mockBaseLlmClient.generateJson).mockResolvedValue(
mockApiResponse,
@@ -364,21 +317,20 @@ describe('NumericalClassifierStrategy', () => {
);
expect(decision).toEqual({
model: PREVIEW_GEMINI_FLASH_MODEL, // Score 40 < Default A/B Threshold 50
model: PREVIEW_GEMINI_FLASH_MODEL, // Score 80 < Default Threshold 90
metadata: {
source: 'NumericalClassifier (Control)',
source: 'NumericalClassifier (Default)',
latencyMs: expect.any(Number),
reasoning: expect.stringContaining('Score: 40 / Threshold: 50'),
reasoning: expect.stringContaining('Score: 80 / Threshold: 90'),
},
});
});
it('should fall back to A/B testing if CLASSIFIER_THRESHOLD is out of range (less than 0)', async () => {
it('should fall back to default logic if CLASSIFIER_THRESHOLD is out of range (less than 0)', async () => {
vi.mocked(mockConfig.getClassifierThreshold).mockResolvedValue(-10);
vi.mocked(mockConfig.getSessionId).mockReturnValue('control-group-id');
const mockApiResponse = {
complexity_reasoning: 'Test task',
complexity_score: 40,
complexity_score: 80,
};
vi.mocked(mockBaseLlmClient.generateJson).mockResolvedValue(
mockApiResponse,
@@ -394,19 +346,18 @@ describe('NumericalClassifierStrategy', () => {
expect(decision).toEqual({
model: PREVIEW_GEMINI_FLASH_MODEL,
metadata: {
source: 'NumericalClassifier (Control)',
source: 'NumericalClassifier (Default)',
latencyMs: expect.any(Number),
reasoning: expect.stringContaining('Score: 40 / Threshold: 50'),
reasoning: expect.stringContaining('Score: 80 / Threshold: 90'),
},
});
});
it('should fall back to A/B testing if CLASSIFIER_THRESHOLD is out of range (greater than 100)', async () => {
it('should fall back to default logic if CLASSIFIER_THRESHOLD is out of range (greater than 100)', async () => {
vi.mocked(mockConfig.getClassifierThreshold).mockResolvedValue(110);
vi.mocked(mockConfig.getSessionId).mockReturnValue('control-group-id');
const mockApiResponse = {
complexity_reasoning: 'Test task',
complexity_score: 60,
complexity_score: 95,
};
vi.mocked(mockBaseLlmClient.generateJson).mockResolvedValue(
mockApiResponse,
@@ -422,9 +373,9 @@ describe('NumericalClassifierStrategy', () => {
expect(decision).toEqual({
model: PREVIEW_GEMINI_MODEL,
metadata: {
source: 'NumericalClassifier (Control)',
source: 'NumericalClassifier (Default)',
latencyMs: expect.any(Number),
reasoning: expect.stringContaining('Score: 60 / Threshold: 50'),
reasoning: expect.stringContaining('Score: 95 / Threshold: 90'),
},
});
});
@@ -591,7 +542,7 @@ describe('NumericalClassifierStrategy', () => {
vi.mocked(mockConfig.getGemini31Launched).mockResolvedValue(true);
const mockApiResponse = {
complexity_reasoning: 'Complex task',
complexity_score: 80,
complexity_score: 95,
};
vi.mocked(mockBaseLlmClient.generateJson).mockResolvedValue(
mockApiResponse,
@@ -613,7 +564,7 @@ describe('NumericalClassifierStrategy', () => {
});
const mockApiResponse = {
complexity_reasoning: 'Complex task',
complexity_score: 80,
complexity_score: 95,
};
vi.mocked(mockBaseLlmClient.generateJson).mockResolvedValue(
mockApiResponse,
@@ -636,7 +587,7 @@ describe('NumericalClassifierStrategy', () => {
});
const mockApiResponse = {
complexity_reasoning: 'Complex task',
complexity_score: 80,
complexity_score: 95,
};
vi.mocked(mockBaseLlmClient.generateJson).mockResolvedValue(
mockApiResponse,
@@ -93,39 +93,6 @@ const ClassifierResponseSchema = z.object({
complexity_score: z.number().min(1).max(100),
});
/**
* Deterministically calculates the routing threshold based on the session ID.
* This ensures a consistent experience for the user within a session.
*
* This implementation uses the FNV-1a hash algorithm (32-bit).
* @see https://en.wikipedia.org/wiki/Fowler%E2%80%93Noll%E2%80%93Vo_hash_function
*
* @param sessionId The unique session identifier.
* @returns The threshold (50 or 80).
*/
function getComplexityThreshold(sessionId: string): number {
const FNV_OFFSET_BASIS_32 = 0x811c9dc5;
const FNV_PRIME_32 = 0x01000193;
let hash = FNV_OFFSET_BASIS_32;
for (let i = 0; i < sessionId.length; i++) {
hash ^= sessionId.charCodeAt(i);
// Multiply by prime (simulate 32-bit overflow with bitwise shift)
hash = Math.imul(hash, FNV_PRIME_32);
}
// Ensure positive integer
hash = hash >>> 0;
// Normalize to 0-99
const normalized = hash % 100;
// 50% split:
// 0-49: Strict (80)
// 50-99: Control (50)
return normalized < 50 ? 80 : 50;
}
export class NumericalClassifierStrategy implements RoutingStrategy {
readonly name = 'numerical_classifier';
@@ -179,11 +146,7 @@ export class NumericalClassifierStrategy implements RoutingStrategy {
const score = routerResponse.complexity_score;
const { threshold, groupLabel, modelAlias } =
await this.getRoutingDecision(
score,
config,
config.getSessionId() || 'unknown-session',
);
await this.getRoutingDecision(score, config);
const [useGemini3_1, useCustomToolModel] = await Promise.all([
config.getGemini31Launched(),
config.getUseCustomToolModel(),
@@ -214,29 +177,19 @@ export class NumericalClassifierStrategy implements RoutingStrategy {
private async getRoutingDecision(
score: number,
config: Config,
sessionId: string,
): Promise<{
threshold: number;
groupLabel: string;
modelAlias: typeof FLASH_MODEL | typeof PRO_MODEL;
}> {
let threshold: number;
let groupLabel: string;
const threshold = await config.getResolvedClassifierThreshold();
const remoteThresholdValue = await config.getClassifierThreshold();
if (
remoteThresholdValue !== undefined &&
!isNaN(remoteThresholdValue) &&
remoteThresholdValue >= 0 &&
remoteThresholdValue <= 100
) {
threshold = remoteThresholdValue;
let groupLabel: string;
if (threshold === remoteThresholdValue) {
groupLabel = 'Remote';
} else {
// Fallback to deterministic A/B test
threshold = getComplexityThreshold(sessionId);
groupLabel = threshold === 80 ? 'Strict' : 'Control';
groupLabel = 'Default';
}
const modelAlias = score >= threshold ? PRO_MODEL : FLASH_MODEL;
@@ -370,6 +370,36 @@ describe('ChatRecordingService', () => {
expect(geminiMsg.toolCalls![0].name).toBe('testTool');
});
it('should preserve dynamic description and NOT overwrite with generic one', () => {
chatRecordingService.recordMessage({
type: 'gemini',
content: '',
model: 'gemini-pro',
});
const dynamicDescription = 'DYNAMIC DESCRIPTION (e.g. Read file foo.txt)';
const toolCall: ToolCallRecord = {
id: 'tool-1',
name: 'testTool',
args: {},
status: CoreToolCallStatus.Success,
timestamp: new Date().toISOString(),
description: dynamicDescription,
};
chatRecordingService.recordToolCalls('gemini-pro', [toolCall]);
const sessionFile = chatRecordingService.getConversationFilePath()!;
const conversation = JSON.parse(
fs.readFileSync(sessionFile, 'utf8'),
) as ConversationRecord;
const geminiMsg = conversation.messages[0] as MessageRecord & {
type: 'gemini';
};
expect(geminiMsg.toolCalls![0].description).toBe(dynamicDescription);
});
it('should create a new message if the last message is not from gemini', () => {
chatRecordingService.recordMessage({
type: 'user',
@@ -347,7 +347,8 @@ export class ChatRecordingService {
return {
...toolCall,
displayName: toolInstance?.displayName || toolCall.name,
description: toolInstance?.description || '',
description:
toolCall.description?.trim() || toolInstance?.description || '',
renderOutputAsMarkdown: toolInstance?.isOutputMarkdown || false,
};
});
@@ -27,6 +27,7 @@ import type {
InvalidChunkEvent,
ContentRetryEvent,
ContentRetryFailureEvent,
NetworkRetryAttemptEvent,
ExtensionInstallEvent,
ToolOutputTruncatedEvent,
ExtensionUninstallEvent,
@@ -94,6 +95,7 @@ export enum EventNames {
INVALID_CHUNK = 'invalid_chunk',
CONTENT_RETRY = 'content_retry',
CONTENT_RETRY_FAILURE = 'content_retry_failure',
RETRY_ATTEMPT = 'retry_attempt',
EXTENSION_ENABLE = 'extension_enable',
EXTENSION_DISABLE = 'extension_disable',
EXTENSION_INSTALL = 'extension_install',
@@ -1231,6 +1233,32 @@ export class ClearcutLogger {
this.flushIfNeeded();
}
logNetworkRetryAttemptEvent(event: NetworkRetryAttemptEvent): void {
// This event is generic for any retry attempt (Gemini, WebFetch, etc.)
const data: EventValue[] = [
{
gemini_cli_key:
EventMetadataKey.GEMINI_CLI_NETWORK_RETRY_ATTEMPT_NUMBER,
value: String(event.attempt),
},
{
gemini_cli_key: EventMetadataKey.GEMINI_CLI_NETWORK_RETRY_DELAY_MS,
value: String(event.delay_ms),
},
{
gemini_cli_key: EventMetadataKey.GEMINI_CLI_NETWORK_RETRY_ERROR_TYPE,
value: event.error_type,
},
{
gemini_cli_key: EventMetadataKey.GEMINI_CLI_API_REQUEST_MODEL,
value: event.model,
},
];
this.enqueueLogEvent(this.createLogEvent(EventNames.RETRY_ATTEMPT, data));
this.flushIfNeeded();
}
async logExtensionInstallEvent(event: ExtensionInstallEvent): Promise<void> {
const data: EventValue[] = [
{
@@ -674,4 +674,17 @@ export enum EventMetadataKey {
// Logs the error message for Conseca events.
CONSECA_ERROR = 166,
// ==========================================================================
// Network Retry Event Keys
// ==========================================================================
// Logs the attempt number for a network retry.
GEMINI_CLI_NETWORK_RETRY_ATTEMPT_NUMBER = 180,
// Logs the delay in milliseconds for a network retry.
GEMINI_CLI_NETWORK_RETRY_DELAY_MS = 181,
// Logs the error type for a network retry.
GEMINI_CLI_NETWORK_RETRY_ERROR_TYPE = 182,
}
+3
View File
@@ -46,6 +46,7 @@ export {
logExtensionUninstall,
logExtensionUpdateEvent,
logWebFetchFallbackAttempt,
logNetworkRetryAttempt,
logRewind,
} from './loggers.js';
export {
@@ -66,6 +67,7 @@ export {
ConversationFinishedEvent,
ToolOutputTruncatedEvent,
WebFetchFallbackAttemptEvent,
NetworkRetryAttemptEvent,
ToolCallDecision,
RewindEvent,
ConsecaPolicyGenerationEvent,
@@ -111,6 +113,7 @@ export {
recordApiErrorMetrics,
recordFileOperationMetric,
recordInvalidChunk,
recordRetryAttemptMetrics,
recordContentRetry,
recordContentRetryFailure,
recordModelRoutingMetrics,
@@ -45,6 +45,7 @@ import {
logAgentStart,
logAgentFinish,
logWebFetchFallbackAttempt,
logNetworkRetryAttempt,
logExtensionUpdateEvent,
logHookCall,
} from './loggers.js';
@@ -70,6 +71,7 @@ import {
EVENT_AGENT_FINISH,
EVENT_WEB_FETCH_FALLBACK_ATTEMPT,
EVENT_INVALID_CHUNK,
EVENT_NETWORK_RETRY_ATTEMPT,
ApiErrorEvent,
ApiRequestEvent,
ApiResponseEvent,
@@ -91,6 +93,7 @@ import {
AgentStartEvent,
AgentFinishEvent,
WebFetchFallbackAttemptEvent,
NetworkRetryAttemptEvent,
ExtensionUpdateEvent,
EVENT_EXTENSION_UPDATE,
HookCallEvent,
@@ -2429,6 +2432,56 @@ describe('loggers', () => {
});
});
describe('logNetworkRetryAttempt', () => {
const mockConfig = makeFakeConfig();
beforeEach(() => {
vi.spyOn(ClearcutLogger.prototype, 'logNetworkRetryAttemptEvent');
vi.spyOn(metrics, 'recordRetryAttemptMetrics');
});
it('logs the network retry attempt event to Clearcut and OTEL', () => {
const event = new NetworkRetryAttemptEvent(
2,
5,
'Overloaded',
1000,
'test-model',
);
logNetworkRetryAttempt(mockConfig, event);
expect(
ClearcutLogger.prototype.logNetworkRetryAttemptEvent,
).toHaveBeenCalledWith(event);
expect(mockLogger.emit).toHaveBeenCalledWith({
body: 'Network retry attempt 2/5 for test-model. Delay: 1000ms. Error type: Overloaded',
attributes: {
'session.id': 'test-session-id',
'user.email': 'test-user@example.com',
'installation.id': 'test-installation-id',
'event.name': EVENT_NETWORK_RETRY_ATTEMPT,
'event.timestamp': '2025-01-01T00:00:00.000Z',
interactive: false,
attempt: 2,
max_attempts: 5,
error_type: 'Overloaded',
delay_ms: 1000,
model: 'test-model',
},
});
expect(metrics.recordRetryAttemptMetrics).toHaveBeenCalledWith(
mockConfig,
{
model: 'test-model',
attempt: 2,
},
);
});
});
describe('Telemetry Buffering', () => {
it('should buffer events when SDK is not initialized', async () => {
vi.spyOn(sdk, 'isTelemetrySdkInitialized').mockReturnValue(false);
+21
View File
@@ -32,6 +32,7 @@ import {
type InvalidChunkEvent,
type ContentRetryEvent,
type ContentRetryFailureEvent,
type NetworkRetryAttemptEvent,
type RipgrepFallbackEvent,
type ToolOutputTruncatedEvent,
type ModelRoutingEvent,
@@ -62,6 +63,7 @@ import {
recordToolCallMetrics,
recordChatCompressionMetrics,
recordFileOperationMetric,
recordRetryAttemptMetrics,
recordContentRetry,
recordContentRetryFailure,
recordModelRoutingMetrics,
@@ -485,6 +487,25 @@ export function logInvalidChunk(
});
}
export function logNetworkRetryAttempt(
config: Config,
event: NetworkRetryAttemptEvent,
): void {
ClearcutLogger.getInstance(config)?.logNetworkRetryAttemptEvent(event);
bufferTelemetryEvent(() => {
const logger = logs.getLogger(SERVICE_NAME);
const logRecord: LogRecord = {
body: event.toLogBody(),
attributes: event.toOpenTelemetryAttributes(config),
};
logger.emit(logRecord);
recordRetryAttemptMetrics(config, {
model: event.model,
attempt: event.attempt,
});
});
}
export function logContentRetry(
config: Config,
event: ContentRetryEvent,
+26
View File
@@ -40,6 +40,7 @@ const INVALID_CHUNK_COUNT = 'gemini_cli.chat.invalid_chunk.count';
const CONTENT_RETRY_COUNT = 'gemini_cli.chat.content_retry.count';
const CONTENT_RETRY_FAILURE_COUNT =
'gemini_cli.chat.content_retry_failure.count';
const NETWORK_RETRY_COUNT = 'gemini_cli.network_retry.count';
const MODEL_ROUTING_LATENCY = 'gemini_cli.model_routing.latency';
const MODEL_ROUTING_FAILURE_COUNT = 'gemini_cli.model_routing.failure.count';
const MODEL_SLASH_COMMAND_CALL_COUNT =
@@ -166,6 +167,16 @@ const COUNTER_DEFINITIONS = {
assign: (c: Counter) => (contentRetryFailureCounter = c),
attributes: {} as Record<string, never>,
},
[NETWORK_RETRY_COUNT]: {
description: 'Counts network retries.',
valueType: ValueType.INT,
assign: (c: Counter) => (networkRetryCounter = c),
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
attributes: {} as {
model: string;
attempt: number;
},
},
[MODEL_ROUTING_FAILURE_COUNT]: {
description: 'Counts model routing failures.',
valueType: ValueType.INT,
@@ -610,6 +621,7 @@ let chatCompressionCounter: Counter | undefined;
let invalidChunkCounter: Counter | undefined;
let contentRetryCounter: Counter | undefined;
let contentRetryFailureCounter: Counter | undefined;
let networkRetryCounter: Counter | undefined;
let modelRoutingLatencyHistogram: Histogram | undefined;
let modelRoutingFailureCounter: Counter | undefined;
let modelSlashCommandCallCounter: Counter | undefined;
@@ -848,6 +860,20 @@ export function recordInvalidChunk(config: Config): void {
invalidChunkCounter.add(1, baseMetricDefinition.getCommonAttributes(config));
}
export function recordRetryAttemptMetrics(
config: Config,
attributes: {
model: string;
attempt: number;
},
): void {
if (!networkRetryCounter || !isMetricsInitialized) return;
networkRetryCounter.add(1, {
...baseMetricDefinition.getCommonAttributes(config),
...attributes,
});
}
/**
* Records a metric for when a retry is triggered due to a content error.
*/
+45
View File
@@ -1341,6 +1341,51 @@ export class ContentRetryEvent implements BaseTelemetryEvent {
export const EVENT_CONTENT_RETRY_FAILURE =
'gemini_cli.chat.content_retry_failure';
export const EVENT_NETWORK_RETRY_ATTEMPT = 'gemini_cli.network_retry_attempt';
export class NetworkRetryAttemptEvent implements BaseTelemetryEvent {
'event.name': 'network_retry_attempt';
'event.timestamp': string;
attempt: number;
max_attempts: number;
error_type: string;
delay_ms: number;
model: string;
constructor(
attempt: number,
max_attempts: number,
error_type: string,
delay_ms: number,
model: string,
) {
this['event.name'] = 'network_retry_attempt';
this['event.timestamp'] = new Date().toISOString();
this.attempt = attempt;
this.max_attempts = max_attempts;
this.error_type = error_type;
this.delay_ms = delay_ms;
this.model = model;
}
toOpenTelemetryAttributes(config: Config): LogAttributes {
return {
...getCommonAttributes(config),
'event.name': EVENT_NETWORK_RETRY_ATTEMPT,
'event.timestamp': this['event.timestamp'],
attempt: this.attempt,
max_attempts: this.max_attempts,
error_type: this.error_type,
delay_ms: this.delay_ms,
model: this.model,
};
}
toLogBody(): string {
return `Network retry attempt ${this.attempt}/${this.max_attempts} for ${this.model}. Delay: ${this.delay_ms}ms. Error type: ${this.error_type}`;
}
}
export class ContentRetryFailureEvent implements BaseTelemetryEvent {
'event.name': 'content_retry_failure';
'event.timestamp': string;
+21 -4
View File
@@ -27,12 +27,14 @@ import { convert } from 'html-to-text';
import {
logWebFetchFallbackAttempt,
WebFetchFallbackAttemptEvent,
logNetworkRetryAttempt,
NetworkRetryAttemptEvent,
} from '../telemetry/index.js';
import { LlmRole } from '../telemetry/llmRole.js';
import { WEB_FETCH_TOOL_NAME } from './tool-names.js';
import { debugLogger } from '../utils/debugLogger.js';
import { coreEvents } from '../utils/events.js';
import { retryWithBackoff } from '../utils/retry.js';
import { retryWithBackoff, getRetryErrorType } from '../utils/retry.js';
import { WEB_FETCH_DEFINITION } from './definitions/coreTools.js';
import { resolveToolDeclaration } from './definitions/resolver.js';
import { LRUCache } from 'mnemonist';
@@ -188,13 +190,28 @@ class WebFetchToolInvocation extends BaseToolInvocation<
}
private handleRetry(attempt: number, error: unknown, delayMs: number): void {
const maxAttempts = this.config.getMaxAttempts();
const modelName = 'Web Fetch';
const errorType = getRetryErrorType(error);
coreEvents.emitRetryAttempt({
attempt,
maxAttempts: this.config.getMaxAttempts(),
maxAttempts,
delayMs,
error: error instanceof Error ? error.message : String(error),
model: 'Web Fetch',
error: errorType,
model: modelName,
});
logNetworkRetryAttempt(
this.config,
new NetworkRetryAttemptEvent(
attempt,
maxAttempts,
errorType,
delayMs,
modelName,
),
);
}
private async executeFallback(signal: AbortSignal): Promise<ToolResult> {
+16
View File
@@ -528,6 +528,22 @@ describe('resolveToRealPath', () => {
expect(resolveToRealPath(input)).toBe(expected);
});
it('should return decoded path even if fs.realpathSync fails with EISDIR', () => {
vi.spyOn(fs, 'realpathSync').mockImplementationOnce(() => {
const err = new Error(
'Illegal operation on a directory',
) as NodeJS.ErrnoException;
err.code = 'EISDIR';
throw err;
});
const p = path.resolve('path', 'to', 'New Project');
const input = pathToFileURL(p).toString();
const expected = p;
expect(resolveToRealPath(input)).toBe(expected);
});
it('should recursively resolve symlinks for non-existent child paths', () => {
const parentPath = path.resolve('/some/parent/path');
const resolvedParentPath = path.resolve('/resolved/parent/path');
+7 -2
View File
@@ -384,7 +384,12 @@ function robustRealpath(p: string, visited = new Set<string>()): string {
try {
return fs.realpathSync(p);
} catch (e: unknown) {
if (e && typeof e === 'object' && 'code' in e && e.code === 'ENOENT') {
if (
e &&
typeof e === 'object' &&
'code' in e &&
(e.code === 'ENOENT' || e.code === 'EISDIR')
) {
try {
const stat = fs.lstatSync(p);
if (stat.isSymbolicLink()) {
@@ -400,7 +405,7 @@ function robustRealpath(p: string, visited = new Set<string>()): string {
lstatError &&
typeof lstatError === 'object' &&
'code' in lstatError &&
lstatError.code === 'ENOENT'
(lstatError.code === 'ENOENT' || lstatError.code === 'EISDIR')
)
) {
throw lstatError;
+40 -2
View File
@@ -99,8 +99,46 @@ function getNetworkErrorCode(error: unknown): string | undefined {
return undefined;
}
const FETCH_FAILED_MESSAGE = 'fetch failed';
const INCOMPLETE_JSON_MESSAGE = 'incomplete json segment';
export const FETCH_FAILED_MESSAGE = 'fetch failed';
export const INCOMPLETE_JSON_MESSAGE = 'incomplete json segment';
/**
* Categorizes an error for retry telemetry purposes.
* Returns a safe string without PII.
*/
export function getRetryErrorType(error: unknown): string {
if (error === 'Invalid content') {
return 'INVALID_CONTENT';
}
const errorCode = getNetworkErrorCode(error);
if (errorCode && RETRYABLE_NETWORK_CODES.includes(errorCode)) {
return errorCode;
}
if (error instanceof Error) {
const lowerMessage = error.message.toLowerCase();
if (lowerMessage.includes(FETCH_FAILED_MESSAGE)) {
return 'FETCH_FAILED';
}
if (lowerMessage.includes(INCOMPLETE_JSON_MESSAGE)) {
return 'INCOMPLETE_JSON';
}
}
const status = getErrorStatus(error);
if (status !== undefined) {
if (status === 429) return 'QUOTA_EXCEEDED';
if (status >= 500 && status < 600) return 'SERVER_ERROR';
return `HTTP_${status}`;
}
if (error instanceof Error) {
return error.name;
}
return 'UNKNOWN';
}
/**
* Default predicate function to determine if a retry should be attempted.