diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index de639f95cf..9103f3c036 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -146,7 +146,7 @@ their corresponding top-level category object in your `settings.json` file. - **`general.retryFetchErrors`** (boolean): - **Description:** Retry on "exception TypeError: fetch failed sending request" errors. - - **Default:** `false` + - **Default:** `true` - **`general.debugKeystrokeLogging`** (boolean): - **Description:** Enable debug logging of keystrokes to the console. diff --git a/eslint.config.js b/eslint.config.js index e2a7fa3e8a..12dc29a238 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -201,6 +201,7 @@ export default tseslint.config( rules: { '@typescript-eslint/no-unsafe-type-assertion': 'error', '@typescript-eslint/no-unsafe-assignment': 'error', + '@typescript-eslint/no-unsafe-return': 'error', }, }, { diff --git a/packages/a2a-server/src/agent/task.test.ts b/packages/a2a-server/src/agent/task.test.ts index 9b5bca8c5c..39cfe5eb74 100644 --- a/packages/a2a-server/src/agent/task.test.ts +++ b/packages/a2a-server/src/agent/task.test.ts @@ -14,19 +14,21 @@ import { type Mock, } from 'vitest'; import { Task } from './task.js'; +import type { + ToolCall, + Config, + ToolCallRequestInfo, + GitService, + CompletedToolCall, +} from '@google/gemini-cli-core'; import { GeminiEventType, - type Config, - type ToolCallRequestInfo, - type GitService, - type CompletedToolCall, ApprovalMode, ToolConfirmationOutcome, } from '@google/gemini-cli-core'; import { createMockConfig } from '../utils/testing_utils.js'; import type { ExecutionEventBus, RequestContext } from '@a2a-js/sdk/server'; import { CoderAgentEvent } from '../types.js'; -import type { ToolCall } from '@google/gemini-cli-core'; const mockProcessRestorableToolCalls = vi.hoisted(() => vi.fn()); diff --git a/packages/a2a-server/src/agent/task.ts b/packages/a2a-server/src/agent/task.ts index 890bc85b11..b74381714d 100644 --- a/packages/a2a-server/src/agent/task.ts +++ b/packages/a2a-server/src/agent/task.ts @@ -30,8 +30,7 @@ import { EDIT_TOOL_NAMES, processRestorableToolCalls, } from '@google/gemini-cli-core'; -import type { RequestContext } from '@a2a-js/sdk/server'; -import { type ExecutionEventBus } from '@a2a-js/sdk/server'; +import type { RequestContext, ExecutionEventBus } from '@a2a-js/sdk/server'; import type { TaskStatusUpdateEvent, TaskArtifactUpdateEvent, diff --git a/packages/a2a-server/src/config/config.ts b/packages/a2a-server/src/config/config.ts index 48daffbe42..eb92e55f36 100644 --- a/packages/a2a-server/src/config/config.ts +++ b/packages/a2a-server/src/config/config.ts @@ -8,17 +8,19 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import * as dotenv from 'dotenv'; -import type { TelemetryTarget } from '@google/gemini-cli-core'; +import type { + TelemetryTarget, + ConfigParameters, + ExtensionLoader, +} from '@google/gemini-cli-core'; import { AuthType, Config, - type ConfigParameters, FileDiscoveryService, ApprovalMode, loadServerHierarchicalMemory, GEMINI_DIR, DEFAULT_GEMINI_EMBEDDING_MODEL, - type ExtensionLoader, startupProfiler, PREVIEW_GEMINI_MODEL, homedir, diff --git a/packages/a2a-server/src/config/settings.ts b/packages/a2a-server/src/config/settings.ts index 3f4528c3b2..a2b11d0886 100644 --- a/packages/a2a-server/src/config/settings.ts +++ b/packages/a2a-server/src/config/settings.ts @@ -147,7 +147,7 @@ function resolveEnvVarsInObject(obj: T): T { } if (Array.isArray(obj)) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion, @typescript-eslint/no-unsafe-return return obj.map((item) => resolveEnvVarsInObject(item)) as unknown as T; } diff --git a/packages/a2a-server/src/http/app.test.ts b/packages/a2a-server/src/http/app.test.ts index 4eb6b522b2..c863fb1472 100644 --- a/packages/a2a-server/src/http/app.test.ts +++ b/packages/a2a-server/src/http/app.test.ts @@ -4,12 +4,11 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type { Config } from '@google/gemini-cli-core'; -import { - GeminiEventType, - ApprovalMode, - type ToolCallConfirmationDetails, +import type { + Config, + ToolCallConfirmationDetails, } from '@google/gemini-cli-core'; +import { GeminiEventType, ApprovalMode } from '@google/gemini-cli-core'; import type { TaskStatusUpdateEvent, SendStreamingMessageSuccessResponse, diff --git a/packages/cli/src/commands/extensions/utils.ts b/packages/cli/src/commands/extensions/utils.ts index 26e47b912b..78bad54502 100644 --- a/packages/cli/src/commands/extensions/utils.ts +++ b/packages/cli/src/commands/extensions/utils.ts @@ -47,6 +47,7 @@ const defaultRequestConfirmation: RequestConfirmationCallback = async ( message, initial: false, }); + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return response.confirm; }; diff --git a/packages/cli/src/config/extensionRegistryClient.ts b/packages/cli/src/config/extensionRegistryClient.ts index 073b78ad79..bf09aabe77 100644 --- a/packages/cli/src/config/extensionRegistryClient.ts +++ b/packages/cli/src/config/extensionRegistryClient.ts @@ -83,6 +83,7 @@ export class ExtensionRegistryClient { }); // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const results = await fzf.find(query); + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return results.map((r: { item: RegistryExtension }) => r.item); } diff --git a/packages/cli/src/config/extensions/extensionEnablement.ts b/packages/cli/src/config/extensions/extensionEnablement.ts index a619587342..7ae2431ee9 100644 --- a/packages/cli/src/config/extensions/extensionEnablement.ts +++ b/packages/cli/src/config/extensions/extensionEnablement.ts @@ -179,6 +179,7 @@ export class ExtensionEnablementManager { readConfig(): AllExtensionsEnablementConfig { try { const content = fs.readFileSync(this.configFilePath, 'utf-8'); + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return JSON.parse(content); } catch (error) { if ( diff --git a/packages/cli/src/config/extensions/extensionSettings.ts b/packages/cli/src/config/extensions/extensionSettings.ts index 23df066db1..06e4f49db4 100644 --- a/packages/cli/src/config/extensions/extensionSettings.ts +++ b/packages/cli/src/config/extensions/extensionSettings.ts @@ -156,6 +156,7 @@ export async function promptForSetting( name: 'value', message: `${setting.name}\n${setting.description}`, }); + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return response.value; } diff --git a/packages/cli/src/config/extensions/variables.ts b/packages/cli/src/config/extensions/variables.ts index 5a2e0ca457..3a79fc705f 100644 --- a/packages/cli/src/config/extensions/variables.ts +++ b/packages/cli/src/config/extensions/variables.ts @@ -58,6 +58,7 @@ export function recursivelyHydrateStrings( if (Array.isArray(obj)) { // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion return obj.map((item) => + // eslint-disable-next-line @typescript-eslint/no-unsafe-return recursivelyHydrateStrings(item, values), ) as unknown as T; } diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index f0e092b45b..3328e0ad7a 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -302,7 +302,7 @@ const SETTINGS_SCHEMA = { label: 'Retry Fetch Errors', category: 'General', requiresRestart: false, - default: false, + default: true, description: 'Retry on "exception TypeError: fetch failed sending request" errors.', showInDialog: false, diff --git a/packages/cli/src/test-utils/mockCommandContext.ts b/packages/cli/src/test-utils/mockCommandContext.ts index 6bf7aafdbc..8dc5b9930a 100644 --- a/packages/cli/src/test-utils/mockCommandContext.ts +++ b/packages/cli/src/test-utils/mockCommandContext.ts @@ -121,5 +121,6 @@ export const createMockCommandContext = ( return output; }; + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return merge(defaultMocks, overrides); }; diff --git a/packages/cli/src/ui/hooks/useAtCompletion.ts b/packages/cli/src/ui/hooks/useAtCompletion.ts index af827f1b12..8d860bb6ce 100644 --- a/packages/cli/src/ui/hooks/useAtCompletion.ts +++ b/packages/cli/src/ui/hooks/useAtCompletion.ts @@ -170,6 +170,7 @@ async function searchResourceCandidates( const results = await fzf.find(normalizedPattern, { limit: MAX_SUGGESTIONS_TO_SHOW * 3, }); + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return results.map( (result: { item: ResourceSuggestionCandidate }) => result.item.suggestion, ); @@ -193,6 +194,7 @@ async function searchAgentCandidates( const results = await fzf.find(normalizedPattern, { limit: MAX_SUGGESTIONS_TO_SHOW, }); + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return results.map((r: { item: Suggestion }) => r.item); } diff --git a/packages/cli/src/ui/utils/TableRenderer.tsx b/packages/cli/src/ui/utils/TableRenderer.tsx index e57e2f7faa..ab1981762c 100644 --- a/packages/cli/src/ui/utils/TableRenderer.tsx +++ b/packages/cli/src/ui/utils/TableRenderer.tsx @@ -193,7 +193,11 @@ export const TableRenderer: React.FC = ({ const wrappedRows = styledRows.map((row) => wrapAndProcessRow(row)); // Use the TIGHTEST widths that fit the wrapped content + padding - const adjustedWidths = actualColumnWidths.map((w) => w + COLUMN_PADDING); + const adjustedWidths = actualColumnWidths.map( + (w) => + // eslint-disable-next-line @typescript-eslint/no-unsafe-return + w + COLUMN_PADDING, + ); return { wrappedHeaders, wrappedRows, adjustedWidths }; }, [styledHeaders, styledRows, terminalWidth]); diff --git a/packages/cli/src/utils/activityLogger.ts b/packages/cli/src/utils/activityLogger.ts index cf852257a9..9f1d268a91 100644 --- a/packages/cli/src/utils/activityLogger.ts +++ b/packages/cli/src/utils/activityLogger.ts @@ -472,7 +472,7 @@ export class ActivityLogger extends EventEmitter { body, pending: true, }); - // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-type-assertion + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-type-assertion, @typescript-eslint/no-unsafe-return return (oldEnd as any).apply(this, [chunk, ...etc]); }; diff --git a/packages/cli/src/utils/envVarResolver.ts b/packages/cli/src/utils/envVarResolver.ts index 4bd1d6f82f..6e01f67ac7 100644 --- a/packages/cli/src/utils/envVarResolver.ts +++ b/packages/cli/src/utils/envVarResolver.ts @@ -98,6 +98,7 @@ function resolveEnvVarsInObjectInternal( visited.add(obj); // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion const result = obj.map((item) => + // eslint-disable-next-line @typescript-eslint/no-unsafe-return resolveEnvVarsInObjectInternal(item, visited, customEnv), ) as unknown as T; visited.delete(obj); diff --git a/packages/cli/src/utils/gitUtils.ts b/packages/cli/src/utils/gitUtils.ts index c25366f72a..e27673f0fe 100644 --- a/packages/cli/src/utils/gitUtils.ts +++ b/packages/cli/src/utils/gitUtils.ts @@ -83,6 +83,7 @@ export const getLatestGitHubRelease = async ( if (!releaseTag) { throw new Error(`Response did not include tag_name field`); } + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return releaseTag; } catch (_error) { debugLogger.debug( diff --git a/packages/cli/src/utils/jsonoutput.ts b/packages/cli/src/utils/jsonoutput.ts index f600b7e165..7f60c34104 100644 --- a/packages/cli/src/utils/jsonoutput.ts +++ b/packages/cli/src/utils/jsonoutput.ts @@ -40,6 +40,7 @@ export function tryParseJSON(input: string): object | null { if (!Array.isArray(parsed) && Object.keys(parsed).length === 0) return null; + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return parsed; } catch (_err) { return null; diff --git a/packages/cli/src/utils/sessionUtils.test.ts b/packages/cli/src/utils/sessionUtils.test.ts index 29fc5bdff9..8491f748bd 100644 --- a/packages/cli/src/utils/sessionUtils.test.ts +++ b/packages/cli/src/utils/sessionUtils.test.ts @@ -445,9 +445,76 @@ describe('SessionSelector', () => { const sessionSelector = new SessionSelector(config); const sessions = await sessionSelector.listSessions(); + // Should list the session with gemini message expect(sessions.length).toBe(1); expect(sessions[0].id).toBe(sessionIdGeminiOnly); }); + + it('should not list sessions marked as subagent', async () => { + const mainSessionId = randomUUID(); + const subagentSessionId = randomUUID(); + + // Create test session files + const chatsDir = path.join(tmpDir, 'chats'); + await fs.mkdir(chatsDir, { recursive: true }); + + // Main session - should be listed + const mainSession = { + sessionId: mainSessionId, + projectHash: 'test-hash', + startTime: '2024-01-01T10:00:00.000Z', + lastUpdated: '2024-01-01T10:30:00.000Z', + messages: [ + { + type: 'user', + content: 'Hello world', + id: 'msg1', + timestamp: '2024-01-01T10:00:00.000Z', + }, + ], + kind: 'main', + }; + + // Subagent session - should NOT be listed + const subagentSession = { + sessionId: subagentSessionId, + projectHash: 'test-hash', + startTime: '2024-01-01T11:00:00.000Z', + lastUpdated: '2024-01-01T11:30:00.000Z', + messages: [ + { + type: 'user', + content: 'Internal subagent task', + id: 'msg1', + timestamp: '2024-01-01T11:00:00.000Z', + }, + ], + kind: 'subagent', + }; + + await fs.writeFile( + path.join( + chatsDir, + `${SESSION_FILE_PREFIX}2024-01-01T10-00-${mainSessionId.slice(0, 8)}.json`, + ), + JSON.stringify(mainSession, null, 2), + ); + + await fs.writeFile( + path.join( + chatsDir, + `${SESSION_FILE_PREFIX}2024-01-01T11-00-${subagentSessionId.slice(0, 8)}.json`, + ), + JSON.stringify(subagentSession, null, 2), + ); + + const sessionSelector = new SessionSelector(config); + const sessions = await sessionSelector.listSessions(); + + // Should only list the main session + expect(sessions.length).toBe(1); + expect(sessions[0].id).toBe(mainSessionId); + }); }); describe('extractFirstUserMessage', () => { diff --git a/packages/cli/src/utils/sessionUtils.ts b/packages/cli/src/utils/sessionUtils.ts index 559a04dccf..039c1232a2 100644 --- a/packages/cli/src/utils/sessionUtils.ts +++ b/packages/cli/src/utils/sessionUtils.ts @@ -276,6 +276,12 @@ export const getAllSessionFiles = async ( return { fileName: file, sessionInfo: null }; } + // Skip subagent sessions - these are implementation details of a tool call + // and shouldn't be surfaced for resumption in the main agent history. + if (content.kind === 'subagent') { + return { fileName: file, sessionInfo: null }; + } + const firstUserMessage = extractFirstUserMessage(content.messages); const isCurrentSession = currentSessionId ? file.includes(currentSessionId.slice(0, 8)) diff --git a/packages/cli/src/utils/settingsUtils.ts b/packages/cli/src/utils/settingsUtils.ts index 8d313cf688..3fa1d8bd5d 100644 --- a/packages/cli/src/utils/settingsUtils.ts +++ b/packages/cli/src/utils/settingsUtils.ts @@ -374,6 +374,7 @@ export function setPendingSettingValue( // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const newSettings = JSON.parse(JSON.stringify(pendingSettings)); setNestedValue(newSettings, path, value); + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return newSettings; } diff --git a/packages/cli/src/zed-integration/fileSystemService.ts b/packages/cli/src/zed-integration/fileSystemService.ts index fc4672b4b5..1d3c8ad0b8 100644 --- a/packages/cli/src/zed-integration/fileSystemService.ts +++ b/packages/cli/src/zed-integration/fileSystemService.ts @@ -29,6 +29,7 @@ export class AcpFileSystemService implements FileSystemService { sessionId: this.sessionId, }); + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return response.content; } diff --git a/packages/core/src/agents/a2a-client-manager.test.ts b/packages/core/src/agents/a2a-client-manager.test.ts index 2f653ba176..42e31d2405 100644 --- a/packages/core/src/agents/a2a-client-manager.test.ts +++ b/packages/core/src/agents/a2a-client-manager.test.ts @@ -11,12 +11,13 @@ import { } from './a2a-client-manager.js'; import type { AgentCard, Task } from '@a2a-js/sdk'; import type { AuthenticationHandler, Client } from '@a2a-js/sdk/client'; -import { ClientFactory, DefaultAgentCardResolver } from '@a2a-js/sdk/client'; -import { debugLogger } from '../utils/debugLogger.js'; import { + ClientFactory, + DefaultAgentCardResolver, createAuthenticatingFetchWithRetry, ClientFactoryOptions, } from '@a2a-js/sdk/client'; +import { debugLogger } from '../utils/debugLogger.js'; vi.mock('../utils/debugLogger.js', () => ({ debugLogger: { diff --git a/packages/core/src/agents/agentLoader.ts b/packages/core/src/agents/agentLoader.ts index ed648c6191..bdc59de746 100644 --- a/packages/core/src/agents/agentLoader.ts +++ b/packages/core/src/agents/agentLoader.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import yaml from 'js-yaml'; +import { load } from 'js-yaml'; import * as fs from 'node:fs/promises'; import { type Dirent } from 'node:fs'; import * as path from 'node:path'; @@ -262,7 +262,7 @@ export async function parseAgentMarkdown( let rawFrontmatter: unknown; try { - rawFrontmatter = yaml.load(frontmatterStr); + rawFrontmatter = load(frontmatterStr); } catch (error) { throw new AgentLoadError( filePath, diff --git a/packages/core/src/agents/local-executor.test.ts b/packages/core/src/agents/local-executor.test.ts index a9a0697bce..8f7269b784 100644 --- a/packages/core/src/agents/local-executor.test.ts +++ b/packages/core/src/agents/local-executor.test.ts @@ -41,14 +41,15 @@ import type { Config } from '../config/config.js'; import { MockTool } from '../test-utils/mock-tool.js'; import { getDirectoryContextString } from '../utils/environmentContext.js'; import { z } from 'zod'; +import { getErrorMessage } from '../utils/errors.js'; import { promptIdContext } from '../utils/promptIdContext.js'; import { logAgentStart, logAgentFinish, logRecoveryAttempt, } from '../telemetry/loggers.js'; -import { LlmRole } from '../telemetry/types.js'; import { + LlmRole, AgentStartEvent, AgentFinishEvent, RecoveryAttemptEvent, @@ -1250,7 +1251,7 @@ describe('LocalAgentExecutor', () => { ); await expect(executor.run({ goal: 'test' }, signal)).rejects.toThrow( - `Failed to create chat object: ${initError}`, + `Failed to create chat object: ${getErrorMessage(initError)}`, ); // Ensure the error was reported via the activity callback @@ -1258,7 +1259,7 @@ describe('LocalAgentExecutor', () => { expect.objectContaining({ type: 'ERROR', data: expect.objectContaining({ - error: `Error: Failed to create chat object: ${initError}`, + error: `Error: Failed to create chat object: ${getErrorMessage(initError)}`, }), }), ); diff --git a/packages/core/src/agents/local-executor.ts b/packages/core/src/agents/local-executor.ts index e6557785db..513424ad32 100644 --- a/packages/core/src/agents/local-executor.ts +++ b/packages/core/src/agents/local-executor.ts @@ -33,8 +33,8 @@ import { import { AgentStartEvent, AgentFinishEvent, - RecoveryAttemptEvent, LlmRole, + RecoveryAttemptEvent, } from '../telemetry/types.js'; import type { LocalAgentDefinition, @@ -48,6 +48,7 @@ import { DEFAULT_MAX_TURNS, DEFAULT_MAX_TIME_MINUTES, } from './types.js'; +import { getErrorMessage } from '../utils/errors.js'; import { templateString } from './utils.js'; import { DEFAULT_GEMINI_MODEL, isAutoModel } from '../config/models.js'; import type { RoutingContext } from '../routing/routingStrategy.js'; @@ -826,16 +827,19 @@ export class LocalAgentExecutor { systemInstruction, [{ functionDeclarations: tools }], startHistory, + undefined, + undefined, + 'subagent', ); - } catch (error) { + } catch (e: unknown) { await reportError( - error, + e, `Error initializing Gemini chat for agent ${this.definition.name}.`, startHistory, 'startChat', ); // Re-throw as a more specific error after reporting. - throw new Error(`Failed to create chat object: ${error}`); + throw new Error(`Failed to create chat object: ${getErrorMessage(e)}`); } } diff --git a/packages/core/src/agents/local-invocation.test.ts b/packages/core/src/agents/local-invocation.test.ts index cdaa46fd76..91efcd399f 100644 --- a/packages/core/src/agents/local-invocation.test.ts +++ b/packages/core/src/agents/local-invocation.test.ts @@ -5,10 +5,13 @@ */ import { describe, it, expect, vi, beforeEach, type Mocked } from 'vitest'; -import type { LocalAgentDefinition } from './types.js'; +import type { + LocalAgentDefinition, + SubagentActivityEvent, + AgentInputs, +} from './types.js'; import { LocalSubagentInvocation } from './local-invocation.js'; import { LocalAgentExecutor } from './local-executor.js'; -import type { SubagentActivityEvent, AgentInputs } from './types.js'; import { AgentTerminateMode } from './types.js'; import { makeFakeConfig } from '../test-utils/config.js'; import { ToolErrorType } from '../tools/tool-error.js'; diff --git a/packages/core/src/agents/registry.test.ts b/packages/core/src/agents/registry.test.ts index 2068968428..8cc45a9a5a 100644 --- a/packages/core/src/agents/registry.test.ts +++ b/packages/core/src/agents/registry.test.ts @@ -8,7 +8,11 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { AgentRegistry, getModelConfigAlias } from './registry.js'; import { makeFakeConfig } from '../test-utils/config.js'; import type { AgentDefinition, LocalAgentDefinition } from './types.js'; -import type { Config, GeminiCLIExtension } from '../config/config.js'; +import type { + Config, + GeminiCLIExtension, + ConfigParameters, +} from '../config/config.js'; import { debugLogger } from '../utils/debugLogger.js'; import { coreEvents, CoreEvent } from '../utils/events.js'; import { A2AClientManager } from './a2a-client-manager.js'; @@ -22,7 +26,6 @@ import { } from '../config/models.js'; import * as tomlLoader from './agentLoader.js'; import { SimpleExtensionLoader } from '../utils/extensionLoader.js'; -import type { ConfigParameters } from '../config/config.js'; import type { ToolRegistry } from '../tools/tool-registry.js'; import { ThinkingLevel } from '@google/genai'; import type { AcknowledgedAgentsService } from './acknowledgedAgents.js'; diff --git a/packages/core/src/agents/remote-invocation.ts b/packages/core/src/agents/remote-invocation.ts index 41564944ec..ea43c901a2 100644 --- a/packages/core/src/agents/remote-invocation.ts +++ b/packages/core/src/agents/remote-invocation.ts @@ -4,12 +4,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type { ToolConfirmationOutcome } from '../tools/tools.js'; -import { - BaseToolInvocation, - type ToolResult, - type ToolCallConfirmationDetails, +import type { + ToolConfirmationOutcome, + ToolResult, + ToolCallConfirmationDetails, } from '../tools/tools.js'; +import { BaseToolInvocation } from '../tools/tools.js'; import { DEFAULT_QUERY_STRING } from './types.js'; import type { RemoteAgentInputs, diff --git a/packages/core/src/agents/types.ts b/packages/core/src/agents/types.ts index b9994d8b4a..2b83bbf908 100644 --- a/packages/core/src/agents/types.ts +++ b/packages/core/src/agents/types.ts @@ -43,12 +43,12 @@ export const DEFAULT_QUERY_STRING = 'Get Started!'; /** * The default maximum number of conversational turns for an agent. */ -export const DEFAULT_MAX_TURNS = 15; +export const DEFAULT_MAX_TURNS = 30; /** * The default maximum execution time for an agent in minutes. */ -export const DEFAULT_MAX_TIME_MINUTES = 5; +export const DEFAULT_MAX_TIME_MINUTES = 10; /** * Represents the validated input parameters passed to an agent upon invocation. diff --git a/packages/core/src/code_assist/converter.test.ts b/packages/core/src/code_assist/converter.test.ts index 31e66bcd17..21fecec547 100644 --- a/packages/core/src/code_assist/converter.test.ts +++ b/packages/core/src/code_assist/converter.test.ts @@ -14,12 +14,12 @@ import { import type { ContentListUnion, GenerateContentParameters, + Part, } from '@google/genai'; import { GenerateContentResponse, FinishReason, BlockedReason, - type Part, } from '@google/genai'; describe('converter', () => { diff --git a/packages/core/src/code_assist/oauth2.ts b/packages/core/src/code_assist/oauth2.ts index fd2f5ea178..7ee3fbe02e 100644 --- a/packages/core/src/code_assist/oauth2.ts +++ b/packages/core/src/code_assist/oauth2.ts @@ -271,7 +271,7 @@ async function initOauthClient( await triggerPostAuthCallbacks(client.credentials); } else { - const userConsent = await getConsentForOauth('Code Assist login required.'); + const userConsent = await getConsentForOauth(''); if (!userConsent) { throw new FatalCancellationError('Authentication cancelled by user.'); } @@ -281,8 +281,7 @@ async function initOauthClient( coreEvents.emit(CoreEvent.UserFeedback, { severity: 'info', message: - `\n\nCode Assist login required.\n` + - `Attempting to open authentication page in your browser.\n` + + `\n\nAttempting to open authentication page in your browser.\n` + `Otherwise navigate to:\n\n${webLogin.authUrl}\n\n\n`, }); try { @@ -636,6 +635,7 @@ async function fetchCachedCredentials(): Promise< for (const keyFile of pathsToTry) { try { const keyFileString = await fs.readFile(keyFile, 'utf-8'); + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return JSON.parse(keyFileString); } catch (error) { // Log specific error for debugging, but continue trying other paths diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index a899ee045f..1f2d9c17b7 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -20,6 +20,7 @@ import { setGeminiMdFilename as mockSetGeminiMdFilename } from '../tools/memoryT import { DEFAULT_TELEMETRY_TARGET, DEFAULT_OTLP_ENDPOINT, + uiTelemetryService, } from '../telemetry/index.js'; import type { ContentGeneratorConfig } from '../core/contentGenerator.js'; import { @@ -33,7 +34,10 @@ import { ShellTool } from '../tools/shell.js'; import { ReadFileTool } from '../tools/read-file.js'; import { GrepTool } from '../tools/grep.js'; import { RipGrepTool, canUseRipgrep } from '../tools/ripGrep.js'; -import { logRipgrepFallback } from '../telemetry/loggers.js'; +import { + logRipgrepFallback, + logApprovalModeDuration, +} from '../telemetry/loggers.js'; import { RipgrepFallbackEvent } from '../telemetry/types.js'; import { ToolRegistry } from '../tools/tool-registry.js'; import { ACTIVATE_SKILL_TOOL_NAME } from '../tools/tool-names.js'; @@ -130,6 +134,7 @@ vi.mock('../telemetry/loggers.js', async (importOriginal) => { return { ...actual, logRipgrepFallback: vi.fn(), + logApprovalModeDuration: vi.fn(), }; }); @@ -201,14 +206,15 @@ vi.mock('../services/contextManager.js', () => ({ import { BaseLlmClient } from '../core/baseLlmClient.js'; import { tokenLimit } from '../core/tokenLimits.js'; -import { uiTelemetryService } from '../telemetry/index.js'; import { getCodeAssistServer } from '../code_assist/codeAssist.js'; import { getExperiments } from '../code_assist/experiments/experiments.js'; import type { CodeAssistServer } from '../code_assist/server.js'; import { ContextManager } from '../services/contextManager.js'; import { UserTierId } from '../code_assist/types.js'; -import type { ModelConfigService } from '../services/modelConfigService.js'; -import type { ModelConfigServiceConfig } from '../services/modelConfigService.js'; +import type { + ModelConfigService, + ModelConfigServiceConfig, +} from '../services/modelConfigService.js'; import { ExitPlanModeTool } from '../tools/exit-plan-mode.js'; import { EnterPlanModeTool } from '../tools/enter-plan-mode.js'; @@ -952,8 +958,13 @@ describe('Server Config (config.ts)', () => { }); describe('Shell Tool Inactivity Timeout', () => { - it('should default to 300000ms (300 seconds) when not provided', () => { + it('should default to 600000ms (600 seconds) for non-interactive when not provided', () => { const config = new Config(baseParams); + expect(config.getShellToolInactivityTimeout()).toBe(600000); + }); + + it('should default to 300000ms (300 seconds) for interactive when not provided', () => { + const config = new Config({ ...baseParams, interactive: true }); expect(config.getShellToolInactivityTimeout()).toBe(300000); }); @@ -1417,6 +1428,84 @@ describe('setApprovalMode with folder trust', () => { expect(updateSpy).not.toHaveBeenCalled(); }); + describe('approval mode duration logging', () => { + beforeEach(() => { + vi.mocked(logApprovalModeDuration).mockClear(); + }); + + it('should initialize lastModeSwitchTime with performance.now() and log positive duration', () => { + const startTime = 1000; + const endTime = 5000; + const performanceSpy = vi.spyOn(performance, 'now'); + + performanceSpy.mockReturnValueOnce(startTime); + const config = new Config(baseParams); + vi.spyOn(config, 'isTrustedFolder').mockReturnValue(true); + + performanceSpy.mockReturnValueOnce(endTime); + config.setApprovalMode(ApprovalMode.PLAN); + + expect(logApprovalModeDuration).toHaveBeenCalledWith( + config, + expect.objectContaining({ + mode: ApprovalMode.DEFAULT, + duration_ms: endTime - startTime, + }), + ); + performanceSpy.mockRestore(); + }); + + it('should skip logging if duration is zero or negative', () => { + const startTime = 5000; + const endTime = 4000; + const performanceSpy = vi.spyOn(performance, 'now'); + + performanceSpy.mockReturnValueOnce(startTime); + const config = new Config(baseParams); + vi.spyOn(config, 'isTrustedFolder').mockReturnValue(true); + + performanceSpy.mockReturnValueOnce(endTime); + config.setApprovalMode(ApprovalMode.PLAN); + + expect(logApprovalModeDuration).not.toHaveBeenCalled(); + performanceSpy.mockRestore(); + }); + + it('should update lastModeSwitchTime after logging to prevent double counting', () => { + const time1 = 1000; + const time2 = 3000; + const time3 = 6000; + const performanceSpy = vi.spyOn(performance, 'now'); + + performanceSpy.mockReturnValueOnce(time1); + const config = new Config(baseParams); + vi.spyOn(config, 'isTrustedFolder').mockReturnValue(true); + + performanceSpy.mockReturnValueOnce(time2); + config.setApprovalMode(ApprovalMode.PLAN); + expect(logApprovalModeDuration).toHaveBeenCalledWith( + config, + expect.objectContaining({ + mode: ApprovalMode.DEFAULT, + duration_ms: time2 - time1, + }), + ); + + vi.mocked(logApprovalModeDuration).mockClear(); + + performanceSpy.mockReturnValueOnce(time3); + config.setApprovalMode(ApprovalMode.YOLO); + expect(logApprovalModeDuration).toHaveBeenCalledWith( + config, + expect.objectContaining({ + mode: ApprovalMode.PLAN, + duration_ms: time3 - time2, + }), + ); + performanceSpy.mockRestore(); + }); + }); + describe('registerCoreTools', () => { beforeEach(() => { vi.clearAllMocks(); @@ -2114,6 +2203,21 @@ describe('Config Quota & Preview Model Access', () => { expect(config.getHasAccessToPreviewModel()).toBe(true); }); + it('should update hasAccessToPreviewModel to true if quota includes Gemini 3.1 preview model', async () => { + mockCodeAssistServer.retrieveUserQuota.mockResolvedValue({ + buckets: [ + { + modelId: 'gemini-3.1-pro-preview', + remainingAmount: '100', + remainingFraction: 1.0, + }, + ], + }); + + await config.refreshUserQuota(); + expect(config.getHasAccessToPreviewModel()).toBe(true); + }); + it('should update hasAccessToPreviewModel to false if quota does not include preview model', async () => { mockCodeAssistServer.retrieveUserQuota.mockResolvedValue({ buckets: [ diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index fc4f7c2ff7..deeb914ea7 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -68,7 +68,12 @@ import { ideContextStore } from '../ide/ideContext.js'; import { WriteTodosTool } from '../tools/write-todos.js'; import type { FileSystemService } from '../services/fileSystemService.js'; import { StandardFileSystemService } from '../services/fileSystemService.js'; -import { logRipgrepFallback, logFlashFallback } from '../telemetry/loggers.js'; +import { + logRipgrepFallback, + logFlashFallback, + logApprovalModeSwitch, + logApprovalModeDuration, +} from '../telemetry/loggers.js'; import { RipgrepFallbackEvent, FlashFallbackEvent, @@ -103,9 +108,11 @@ import type { EventEmitter } from 'node:events'; import { PolicyEngine } from '../policy/policy-engine.js'; import { ApprovalMode, type PolicyEngineConfig } from '../policy/types.js'; import { HookSystem } from '../hooks/index.js'; -import type { UserTierId } from '../code_assist/types.js'; -import type { RetrieveUserQuotaResponse } from '../code_assist/types.js'; -import type { AdminControlsSettings } from '../code_assist/types.js'; +import type { + UserTierId, + RetrieveUserQuotaResponse, + AdminControlsSettings, +} from '../code_assist/types.js'; import type { HierarchicalMemory } from './memory.js'; import { getCodeAssistServer } from '../code_assist/codeAssist.js'; import type { Experiments } from '../code_assist/experiments/experiments.js'; @@ -119,10 +126,6 @@ import { debugLogger } from '../utils/debugLogger.js'; import { SkillManager, type SkillDefinition } from '../skills/skillManager.js'; import { startupProfiler } from '../telemetry/startupProfiler.js'; import type { AgentDefinition } from '../agents/types.js'; -import { - logApprovalModeSwitch, - logApprovalModeDuration, -} from '../telemetry/loggers.js'; import { fetchAdminControls } from '../code_assist/admin/admin_controls.js'; import { isSubpath } from '../utils/paths.js'; import { UserHintService } from './userHintService.js'; @@ -696,7 +699,7 @@ export class Config { private terminalBackground: string | undefined = undefined; private remoteAdminSettings: AdminControlsSettings | undefined; private latestApiRequest: GenerateContentParameters | undefined; - private lastModeSwitchTime: number = Date.now(); + private lastModeSwitchTime: number = performance.now(); readonly userHintService: UserHintService; private approvedPlanPath: string | undefined; @@ -849,8 +852,9 @@ export class Config { this.continueOnFailedApiCall = params.continueOnFailedApiCall ?? true; this.enableShellOutputEfficiency = params.enableShellOutputEfficiency ?? true; + const defaultShellTimeout = this.interactive ? 300 : 600; // 5 min interactive, 10 min non-interactive this.shellToolInactivityTimeout = - (params.shellToolInactivityTimeout ?? 300) * 1000; // 5 minutes + (params.shellToolInactivityTimeout ?? defaultShellTimeout) * 1000; this.extensionManagement = params.extensionManagement ?? true; this.enableExtensionReloading = params.enableExtensionReloading ?? false; this.storage = new Storage(this.targetDir, this.sessionId); @@ -874,7 +878,7 @@ export class Config { this.outputSettings = { format: params.output?.format ?? OutputFormat.TEXT, }; - this.retryFetchErrors = params.retryFetchErrors ?? false; + this.retryFetchErrors = params.retryFetchErrors ?? true; this.disableYoloMode = params.disableYoloMode ?? false; this.rawOutput = params.rawOutput ?? false; this.acceptRawOutputRisk = params.acceptRawOutputRisk ?? false; @@ -1497,7 +1501,8 @@ export class Config { } const hasAccess = - quota.buckets?.some((b) => b.modelId === PREVIEW_GEMINI_MODEL) ?? false; + quota.buckets?.some((b) => b.modelId && isPreviewModel(b.modelId)) ?? + false; this.setHasAccessToPreviewModel(hasAccess); return quota; } catch (e) { @@ -1795,12 +1800,11 @@ export class Config { const currentMode = this.getApprovalMode(); if (currentMode !== mode) { - this.logCurrentModeDuration(this.getApprovalMode()); + this.logCurrentModeDuration(currentMode); logApprovalModeSwitch( this, new ApprovalModeSwitchEvent(currentMode, mode), ); - this.lastModeSwitchTime = Date.now(); } this.policyEngine.setApprovalMode(mode); @@ -1863,12 +1867,15 @@ export class Config { * Logs the duration of the current approval mode. */ logCurrentModeDuration(mode: ApprovalMode): void { - const now = Date.now(); + const now = performance.now(); const duration = now - this.lastModeSwitchTime; - logApprovalModeDuration( - this, - new ApprovalModeDurationEvent(mode, duration), - ); + if (duration > 0) { + logApprovalModeDuration( + this, + new ApprovalModeDurationEvent(mode, duration), + ); + } + this.lastModeSwitchTime = now; } isYoloModeDisabled(): boolean { diff --git a/packages/core/src/config/models.test.ts b/packages/core/src/config/models.test.ts index c16cf49781..3337151151 100644 --- a/packages/core/src/config/models.test.ts +++ b/packages/core/src/config/models.test.ts @@ -36,6 +36,7 @@ describe('isPreviewModel', () => { it('should return true for preview models', () => { expect(isPreviewModel(PREVIEW_GEMINI_MODEL)).toBe(true); expect(isPreviewModel(PREVIEW_GEMINI_3_1_MODEL)).toBe(true); + expect(isPreviewModel(PREVIEW_GEMINI_3_1_CUSTOM_TOOLS_MODEL)).toBe(true); expect(isPreviewModel(PREVIEW_GEMINI_FLASH_MODEL)).toBe(true); expect(isPreviewModel(PREVIEW_GEMINI_MODEL_AUTO)).toBe(true); }); diff --git a/packages/core/src/config/models.ts b/packages/core/src/config/models.ts index d0ec49f005..54ea063569 100644 --- a/packages/core/src/config/models.ts +++ b/packages/core/src/config/models.ts @@ -134,6 +134,7 @@ export function isPreviewModel(model: string): boolean { return ( model === PREVIEW_GEMINI_MODEL || model === PREVIEW_GEMINI_3_1_MODEL || + model === PREVIEW_GEMINI_3_1_CUSTOM_TOOLS_MODEL || model === PREVIEW_GEMINI_FLASH_MODEL || model === PREVIEW_GEMINI_MODEL_AUTO ); diff --git a/packages/core/src/config/projectRegistry.ts b/packages/core/src/config/projectRegistry.ts index 225faedf9b..725ea081f9 100644 --- a/packages/core/src/config/projectRegistry.ts +++ b/packages/core/src/config/projectRegistry.ts @@ -59,6 +59,7 @@ export class ProjectRegistry { try { const content = await fs.promises.readFile(this.registryPath, 'utf8'); + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return JSON.parse(content); } catch (e) { debugLogger.debug('Failed to load registry: ', e); diff --git a/packages/core/src/core/__snapshots__/prompts.test.ts.snap b/packages/core/src/core/__snapshots__/prompts.test.ts.snap index 4ec2bccd91..c3fceb1492 100644 --- a/packages/core/src/core/__snapshots__/prompts.test.ts.snap +++ b/packages/core/src/core/__snapshots__/prompts.test.ts.snap @@ -799,7 +799,51 @@ Operate using a **Research -> Strategy -> Execution -> Review** lifecycle. For t ## Interaction Details - **Help Command:** The user can use '/help' to display help information. -- **Feedback:** To report a bug or provide feedback, please use the /bug command." +- **Feedback:** To report a bug or provide feedback, please use the /bug command. + +## File Creation (Non-Interactive) +- **Prefer write_file over shell heredocs** for creating or overwriting files. The write_file tool is more reliable and avoids shell escaping issues. +- Only use shell heredocs when you need to append to a file or when the content depends on shell variable expansion. +- If you must use a heredoc, always use the full syntax: \`cat << 'EOF' > filename\` — never omit \`cat\`. + +## Background Services (Non-Interactive) +- To start a service that must persist across turns, use: \`nohup > /tmp/.log 2>&1 &\` +- After starting, verify the process is running: + 1. \`echo $!\` to capture the PID immediately after starting + 2. \`sleep 1 && kill -0 \` to confirm it's still alive + 3. Check the log file for startup errors: \`cat /tmp/.log\` +- If a service fails to start, check the log file before retrying. +- For services that need time to initialize (databases, web servers), add a brief \`sleep 2\` and verify the port is open before proceeding. + +## Error Recovery (Non-Interactive) +- **Don't blindly retry:** When a tool call fails, analyze the error before retrying. Do not immediately retry with the same arguments. +- **Web fetch failures:** If web_fetch fails, try simplifying the prompt or use google_web_search as an alternative to find the information. +- **Shell failures:** Check error codes and run diagnostic commands before retrying. For compilation errors, fix one issue at a time rather than attempting multiple fixes simultaneously. +- **Maximum retries:** Attempt the same approach at most 2 times. If it fails twice, try an alternative strategy or tool. +- **Avoid loops:** If you find yourself repeating the same sequence of actions, stop and reassess your approach. + +## Missing Commands / "command not found" +- When a command is not found, install the package that provides it before retrying. +- Use \`apt-get install -y \` (Debian/Ubuntu) or \`yum install -y \` (RHEL/CentOS). +- Common mappings: + - pgrep, ps, pidof → procps + - ss → iproute2 + - netstat, ifconfig → net-tools + - curl → curl + - jq → jq + - dig, nslookup → dnsutils (or bind-utils on RHEL) + - ip → iproute2 + - lsof → lsof + - wget → wget + - tree → tree + - zip/unzip → zip / unzip +- If you don't know the package name, try \`apt-cache search \` or \`yum provides \`. + +## Process Verification Without pgrep +- If pgrep/ps are unavailable and you can't install them, verify processes using: + - \`ls /proc/[PID]\` to check if a PID is still running + - \`kill -0 \` to test if a process exists (returns 0 if it does) + - \`cat /proc//cmdline\` to inspect what a process is running" `; exports[`Core System Prompt (prompts.ts) > should handle CodebaseInvestigator with tools=grep_search,glob 1`] = ` @@ -926,7 +970,51 @@ Operate using a **Research -> Strategy -> Execution -> Review** lifecycle. For t ## Interaction Details - **Help Command:** The user can use '/help' to display help information. -- **Feedback:** To report a bug or provide feedback, please use the /bug command." +- **Feedback:** To report a bug or provide feedback, please use the /bug command. + +## File Creation (Non-Interactive) +- **Prefer write_file over shell heredocs** for creating or overwriting files. The write_file tool is more reliable and avoids shell escaping issues. +- Only use shell heredocs when you need to append to a file or when the content depends on shell variable expansion. +- If you must use a heredoc, always use the full syntax: \`cat << 'EOF' > filename\` — never omit \`cat\`. + +## Background Services (Non-Interactive) +- To start a service that must persist across turns, use: \`nohup > /tmp/.log 2>&1 &\` +- After starting, verify the process is running: + 1. \`echo $!\` to capture the PID immediately after starting + 2. \`sleep 1 && kill -0 \` to confirm it's still alive + 3. Check the log file for startup errors: \`cat /tmp/.log\` +- If a service fails to start, check the log file before retrying. +- For services that need time to initialize (databases, web servers), add a brief \`sleep 2\` and verify the port is open before proceeding. + +## Error Recovery (Non-Interactive) +- **Don't blindly retry:** When a tool call fails, analyze the error before retrying. Do not immediately retry with the same arguments. +- **Web fetch failures:** If web_fetch fails, try simplifying the prompt or use google_web_search as an alternative to find the information. +- **Shell failures:** Check error codes and run diagnostic commands before retrying. For compilation errors, fix one issue at a time rather than attempting multiple fixes simultaneously. +- **Maximum retries:** Attempt the same approach at most 2 times. If it fails twice, try an alternative strategy or tool. +- **Avoid loops:** If you find yourself repeating the same sequence of actions, stop and reassess your approach. + +## Missing Commands / "command not found" +- When a command is not found, install the package that provides it before retrying. +- Use \`apt-get install -y \` (Debian/Ubuntu) or \`yum install -y \` (RHEL/CentOS). +- Common mappings: + - pgrep, ps, pidof → procps + - ss → iproute2 + - netstat, ifconfig → net-tools + - curl → curl + - jq → jq + - dig, nslookup → dnsutils (or bind-utils on RHEL) + - ip → iproute2 + - lsof → lsof + - wget → wget + - tree → tree + - zip/unzip → zip / unzip +- If you don't know the package name, try \`apt-cache search \` or \`yum provides \`. + +## Process Verification Without pgrep +- If pgrep/ps are unavailable and you can't install them, verify processes using: + - \`ls /proc/[PID]\` to check if a PID is still running + - \`kill -0 \` to test if a process exists (returns 0 if it does) + - \`cat /proc//cmdline\` to inspect what a process is running" `; exports[`Core System Prompt (prompts.ts) > should handle git instructions when isGitRepository=false 1`] = ` diff --git a/packages/core/src/core/baseLlmClient.test.ts b/packages/core/src/core/baseLlmClient.test.ts index 4d09a1edd9..d067ec49ef 100644 --- a/packages/core/src/core/baseLlmClient.test.ts +++ b/packages/core/src/core/baseLlmClient.test.ts @@ -15,22 +15,24 @@ import { type Mock, } from 'vitest'; -import { BaseLlmClient, type GenerateJsonOptions } from './baseLlmClient.js'; +import type { + GenerateContentOptions, + GenerateJsonOptions, +} from './baseLlmClient.js'; +import { BaseLlmClient } from './baseLlmClient.js'; import type { ContentGenerator } from './contentGenerator.js'; import type { ModelAvailabilityService } from '../availability/modelAvailabilityService.js'; import { createAvailabilityServiceMock } from '../availability/testUtils.js'; -import type { GenerateContentOptions } from './baseLlmClient.js'; import type { GenerateContentResponse } from '@google/genai'; import type { Config } from '../config/config.js'; import { AuthType } from './contentGenerator.js'; import { reportError } from '../utils/errorReporting.js'; import { logMalformedJsonResponse } from '../telemetry/loggers.js'; import { retryWithBackoff } from '../utils/retry.js'; -import { MalformedJsonResponseEvent } from '../telemetry/types.js'; +import { MalformedJsonResponseEvent, LlmRole } from '../telemetry/types.js'; import { getErrorMessage } from '../utils/errors.js'; import type { ModelConfigService } from '../services/modelConfigService.js'; import { makeResolvedModelConfig } from '../services/modelConfigServiceTestUtils.js'; -import { LlmRole } from '../telemetry/types.js'; vi.mock('../utils/errorReporting.js'); vi.mock('../telemetry/loggers.js'); diff --git a/packages/core/src/core/baseLlmClient.ts b/packages/core/src/core/baseLlmClient.ts index 64730ff74c..64442ac86e 100644 --- a/packages/core/src/core/baseLlmClient.ts +++ b/packages/core/src/core/baseLlmClient.ts @@ -13,21 +13,19 @@ import type { GenerateContentConfig, } from '@google/genai'; import type { Config } from '../config/config.js'; -import type { ContentGenerator } from './contentGenerator.js'; -import type { AuthType } from './contentGenerator.js'; +import type { ContentGenerator, AuthType } from './contentGenerator.js'; 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 } from '../telemetry/types.js'; +import { MalformedJsonResponseEvent, LlmRole } from '../telemetry/types.js'; import { retryWithBackoff } from '../utils/retry.js'; import type { ModelConfigKey } from '../services/modelConfigService.js'; import { applyModelSelection, createAvailabilityContextProvider, } from '../availability/policyHelpers.js'; -import { LlmRole } from '../telemetry/types.js'; const DEFAULT_MAX_ATTEMPTS = 5; @@ -164,6 +162,7 @@ export class BaseLlmClient { ); // If we are here, the content is valid (not empty and parsable). + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return JSON.parse( this.cleanJsonResponse(getResponseText(result)!.trim(), model), ); diff --git a/packages/core/src/core/client.test.ts b/packages/core/src/core/client.test.ts index c910556ca8..0924645cea 100644 --- a/packages/core/src/core/client.test.ts +++ b/packages/core/src/core/client.test.ts @@ -1207,7 +1207,7 @@ ${JSON.stringify( eventCount++; // Safety check to prevent actual infinite loop in test - if (eventCount > 200) { + if (eventCount > 400) { abortController.abort(); throw new Error( 'Test exceeded expected event limit - possible actual infinite loop', @@ -1219,13 +1219,12 @@ ${JSON.stringify( expect(finalResult).toBeInstanceOf(Turn); // If infinite loop protection is working, checkNextSpeaker should be called many times - // but stop at MAX_TURNS (100). Since each recursive call should trigger checkNextSpeaker, - // we expect it to be called multiple times before hitting the limit + // but stop at maxTurns (200 for non-interactive). Since each recursive call should trigger + // checkNextSpeaker, we expect it to be called multiple times before hitting the limit expect(mockCheckNextSpeaker).toHaveBeenCalled(); // The stream should produce events and eventually terminate expect(eventCount).toBeGreaterThanOrEqual(1); - expect(eventCount).toBeLessThan(200); // Should not exceed our safety limit }); it('should yield MaxSessionTurns and stop when session turn limit is reached', async () => { @@ -1347,9 +1346,9 @@ ${JSON.stringify( const callCount = mockCheckNextSpeaker.mock.calls.length; // With the fix: even when turns is set to a very high value, - // the loop should stop at MAX_TURNS (100) - expect(callCount).toBeLessThanOrEqual(100); // Should not exceed MAX_TURNS - expect(eventCount).toBeLessThanOrEqual(200); // Should have reasonable number of events + // the loop should stop at maxTurns (200 for non-interactive) + expect(callCount).toBeLessThanOrEqual(200); // Should not exceed maxTurns + expect(eventCount).toBeLessThanOrEqual(400); // Should have reasonable number of events }); it('should yield ContextWindowWillOverflow when the context window is about to overflow', async () => { diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index efa35a868b..0afa2d2381 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -17,8 +17,7 @@ import { getInitialChatHistory, } from '../utils/environmentContext.js'; import type { ServerGeminiStreamEvent, ChatCompressionInfo } from './turn.js'; -import { CompressionStatus } from './turn.js'; -import { Turn, GeminiEventType } from './turn.js'; +import { CompressionStatus, Turn, GeminiEventType } from './turn.js'; import type { Config } from '../config/config.js'; import { getCoreSystemPrompt } from './prompts.js'; import { checkNextSpeaker } from '../utils/nextSpeakerChecker.js'; @@ -66,7 +65,8 @@ import { partToString } from '../utils/partUtils.js'; import { coreEvents, CoreEvent } from '../utils/events.js'; import type { LlmRole } from '../telemetry/types.js'; -const MAX_TURNS = 100; +const MAX_TURNS_INTERACTIVE = 100; +const MAX_TURNS_NON_INTERACTIVE = 200; type BeforeAgentHookReturn = | { @@ -87,6 +87,7 @@ export class GeminiClient { private readonly loopDetector: LoopDetectionService; private readonly compressionService: ChatCompressionService; private readonly toolOutputMaskingService: ToolOutputMaskingService; + private readonly maxTurns: number; private lastPromptId: string; private currentSequenceModel: string | null = null; private lastSentIdeContext: IdeContext | undefined; @@ -103,6 +104,9 @@ export class GeminiClient { this.compressionService = new ChatCompressionService(); this.toolOutputMaskingService = new ToolOutputMaskingService(); this.lastPromptId = this.config.getSessionId(); + this.maxTurns = config.isInteractive() + ? MAX_TURNS_INTERACTIVE + : MAX_TURNS_NON_INTERACTIVE; coreEvents.on(CoreEvent.ModelChanged, this.handleModelChanged); } @@ -791,7 +795,7 @@ export class GeminiClient { request: PartListUnion, signal: AbortSignal, prompt_id: string, - turns: number = MAX_TURNS, + turns?: number, isInvalidStreamRetry: boolean = false, displayContent?: PartListUnion, ): AsyncGenerator { @@ -839,7 +843,7 @@ export class GeminiClient { } } - const boundedTurns = Math.min(turns, MAX_TURNS); + const boundedTurns = Math.min(turns ?? this.maxTurns, this.maxTurns); let turn = new Turn(this.getChat(), prompt_id); try { diff --git a/packages/core/src/core/coreToolHookTriggers.ts b/packages/core/src/core/coreToolHookTriggers.ts index 0ed947623c..cb98d3af20 100644 --- a/packages/core/src/core/coreToolHookTriggers.ts +++ b/packages/core/src/core/coreToolHookTriggers.ts @@ -6,11 +6,14 @@ import { type McpToolContext, BeforeToolHookOutput } from '../hooks/types.js'; import type { Config } from '../config/config.js'; -import type { ToolResult, AnyDeclarativeTool } from '../tools/tools.js'; +import type { + ToolResult, + AnyDeclarativeTool, + AnyToolInvocation, +} from '../tools/tools.js'; import { ToolErrorType } from '../tools/tool-error.js'; import { debugLogger } from '../utils/debugLogger.js'; import type { AnsiOutput, ShellExecutionConfig } from '../index.js'; -import type { AnyToolInvocation } from '../tools/tools.js'; import { ShellToolInvocation } from '../tools/shell.js'; import { DiscoveredMCPToolInvocation } from '../tools/mcp-tool.js'; diff --git a/packages/core/src/core/fakeContentGenerator.ts b/packages/core/src/core/fakeContentGenerator.ts index 5bedc2d187..c765bde087 100644 --- a/packages/core/src/core/fakeContentGenerator.ts +++ b/packages/core/src/core/fakeContentGenerator.ts @@ -83,6 +83,7 @@ export class FakeContentGenerator implements ContentGenerator { // eslint-disable-next-line @typescript-eslint/no-unused-vars role: LlmRole, ): Promise { + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return Object.setPrototypeOf( this.getNextResponse('generateContent', request), GenerateContentResponse.prototype, @@ -116,6 +117,7 @@ export class FakeContentGenerator implements ContentGenerator { async embedContent( request: EmbedContentParameters, ): Promise { + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return Object.setPrototypeOf( this.getNextResponse('embedContent', request), EmbedContentResponse.prototype, diff --git a/packages/core/src/core/geminiChat.test.ts b/packages/core/src/core/geminiChat.test.ts index 8a6b3f8bc8..9796e85ccd 100644 --- a/packages/core/src/core/geminiChat.test.ts +++ b/packages/core/src/core/geminiChat.test.ts @@ -1315,11 +1315,11 @@ describe('GeminiChat', () => { } }).rejects.toThrow(InvalidStreamError); - // Should be called 2 times (initial + 1 retry) + // Should be called 3 times (initial + 2 retries) expect(mockContentGenerator.generateContentStream).toHaveBeenCalledTimes( - 2, + 3, ); - expect(mockLogContentRetry).toHaveBeenCalledTimes(1); + expect(mockLogContentRetry).toHaveBeenCalledTimes(2); expect(mockLogContentRetryFailure).toHaveBeenCalledTimes(1); // History should still contain the user message. diff --git a/packages/core/src/core/geminiChat.ts b/packages/core/src/core/geminiChat.ts index 14f90cea9d..5300bfdfbb 100644 --- a/packages/core/src/core/geminiChat.ts +++ b/packages/core/src/core/geminiChat.ts @@ -86,8 +86,8 @@ interface ContentRetryOptions { } const INVALID_CONTENT_RETRY_OPTIONS: ContentRetryOptions = { - maxAttempts: 2, // 1 initial call + 1 retry - initialDelayMs: 500, + maxAttempts: 3, // 1 initial call + 2 retries + initialDelayMs: 1000, }; export const SYNTHETIC_THOUGHT_SIGNATURE = 'skip_thought_signature_validator'; @@ -249,10 +249,11 @@ export class GeminiChat { private history: Content[] = [], resumedSessionData?: ResumedSessionData, private readonly onModelChanged?: (modelId: string) => Promise, + kind: 'main' | 'subagent' = 'main', ) { validateHistory(history); this.chatRecordingService = new ChatRecordingService(config); - this.chatRecordingService.initialize(resumedSessionData); + this.chatRecordingService.initialize(resumedSessionData, kind); this.lastPromptTokenCount = estimateTokenCountSync( this.history.flatMap((c) => c.parts || []), ); diff --git a/packages/core/src/hooks/hookAggregator.ts b/packages/core/src/hooks/hookAggregator.ts index b8a280cca1..5cd53e8c6e 100644 --- a/packages/core/src/hooks/hookAggregator.ts +++ b/packages/core/src/hooks/hookAggregator.ts @@ -17,8 +17,8 @@ import { BeforeToolSelectionHookOutput, AfterModelHookOutput, AfterAgentHookOutput, + HookEventName, } from './types.js'; -import { HookEventName } from './types.js'; /** * Aggregated hook result diff --git a/packages/core/src/hooks/hookEventHandler.test.ts b/packages/core/src/hooks/hookEventHandler.test.ts index b9ae878e76..9a07d39672 100644 --- a/packages/core/src/hooks/hookEventHandler.test.ts +++ b/packages/core/src/hooks/hookEventHandler.test.ts @@ -11,16 +11,16 @@ import type { import { describe, it, expect, vi, beforeEach } from 'vitest'; import { HookEventHandler } from './hookEventHandler.js'; import type { Config } from '../config/config.js'; -import type { HookConfig } from './types.js'; -import type { HookPlanner } from './hookPlanner.js'; -import type { HookRunner } from './hookRunner.js'; -import type { HookAggregator } from './hookAggregator.js'; -import { HookEventName, HookType } from './types.js'; +import type { HookConfig, HookExecutionResult } from './types.js'; import { NotificationType, SessionStartSource, - type HookExecutionResult, + HookEventName, + HookType, } from './types.js'; +import type { HookPlanner } from './hookPlanner.js'; +import type { HookRunner } from './hookRunner.js'; +import type { HookAggregator } from './hookAggregator.js'; // Mock debugLogger const mockDebugLogger = vi.hoisted(() => ({ diff --git a/packages/core/src/hooks/hookPlanner.ts b/packages/core/src/hooks/hookPlanner.ts index 92701c4a42..3e016efe23 100644 --- a/packages/core/src/hooks/hookPlanner.ts +++ b/packages/core/src/hooks/hookPlanner.ts @@ -5,8 +5,8 @@ */ import type { HookRegistry, HookRegistryEntry } from './hookRegistry.js'; -import type { HookExecutionPlan } from './types.js'; -import { getHookKey, type HookEventName } from './types.js'; +import type { HookExecutionPlan, HookEventName } from './types.js'; +import { getHookKey } from './types.js'; import { debugLogger } from '../utils/debugLogger.js'; /** diff --git a/packages/core/src/hooks/hookRunner.test.ts b/packages/core/src/hooks/hookRunner.test.ts index 5bc671b088..ca88b9411e 100644 --- a/packages/core/src/hooks/hookRunner.test.ts +++ b/packages/core/src/hooks/hookRunner.test.ts @@ -8,8 +8,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process'; import { HookRunner } from './hookRunner.js'; import { HookEventName, HookType, ConfigSource } from './types.js'; -import type { HookConfig } from './types.js'; -import type { HookInput } from './types.js'; +import type { HookConfig, HookInput } from './types.js'; import type { Readable, Writable } from 'node:stream'; import type { Config } from '../config/config.js'; diff --git a/packages/core/src/ide/ide-client.ts b/packages/core/src/ide/ide-client.ts index 2b3b29ac3c..373df31f5f 100644 --- a/packages/core/src/ide/ide-client.ts +++ b/packages/core/src/ide/ide-client.ts @@ -348,6 +348,7 @@ export class IdeClient { // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const parsedJson = JSON.parse(textPart.text); if (parsedJson && typeof parsedJson.content === 'string') { + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return parsedJson.content; } if (parsedJson && parsedJson.content === null) { diff --git a/packages/core/src/ide/ide-connection-utils.ts b/packages/core/src/ide/ide-connection-utils.ts index 81a9740327..c9776e1509 100644 --- a/packages/core/src/ide/ide-connection-utils.ts +++ b/packages/core/src/ide/ide-connection-utils.ts @@ -123,6 +123,7 @@ export async function getConnectionConfigFromFile( `gemini-ide-server-${pid}.json`, ); const portFileContents = await fs.promises.readFile(portFile, 'utf8'); + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return JSON.parse(portFileContents); } catch (_) { // For newer extension versions, the file name matches the pattern @@ -167,6 +168,7 @@ export async function getConnectionConfigFromFile( } const parsedContents = fileContents.map((content) => { try { + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return JSON.parse(content); } catch (e) { logger.debug('Failed to parse JSON from config file: ', e); @@ -196,6 +198,7 @@ export async function getConnectionConfigFromFile( if (fileIndex !== -1) { logger.debug(`Selected IDE connection file: ${matchingFiles[fileIndex]}`); } + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return selected; } @@ -213,6 +216,7 @@ export async function getConnectionConfigFromFile( `Selected IDE connection file (matched port from env): ${matchingFiles[fileIndex]}`, ); } + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return selected; } } @@ -225,6 +229,7 @@ export async function getConnectionConfigFromFile( `Selected first valid IDE connection file: ${matchingFiles[fileIndex]}`, ); } + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return selected; } diff --git a/packages/core/src/ide/ide-installer.test.ts b/packages/core/src/ide/ide-installer.test.ts index e35cb3280f..0347fd892f 100644 --- a/packages/core/src/ide/ide-installer.test.ts +++ b/packages/core/src/ide/ide-installer.test.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { vi } from 'vitest'; +import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'; vi.mock('node:child_process', async (importOriginal) => { const actual = await importOriginal(); @@ -24,7 +24,6 @@ vi.mock('../utils/paths.js', async (importOriginal) => { }; }); -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { getIdeInstaller } from './ide-installer.js'; import * as child_process from 'node:child_process'; import * as fs from 'node:fs'; diff --git a/packages/core/src/prompts/snippets.ts b/packages/core/src/prompts/snippets.ts index 390ff98c9b..6c62e68244 100644 --- a/packages/core/src/prompts/snippets.ts +++ b/packages/core/src/prompts/snippets.ts @@ -338,6 +338,9 @@ export function renderOperationalGuidelines( ## Interaction Details - **Help Command:** The user can use '/help' to display help information. - **Feedback:** To report a bug or provide feedback, please use the /bug command. +${!options.interactive ? nonInteractiveFileWritingGuidance() : ''} +${!options.interactive ? nonInteractiveProcessGuidance() : ''} +${!options.interactive ? nonInteractiveErrorRecovery() : ''} `.trim(); } @@ -679,6 +682,59 @@ function gitRepoKeepUserInformed(interactive: boolean): string { : ''; } +function nonInteractiveFileWritingGuidance(): string { + return ` +## File Creation (Non-Interactive) +- **Prefer ${WRITE_FILE_TOOL_NAME} over shell heredocs** for creating or overwriting files. The ${WRITE_FILE_TOOL_NAME} tool is more reliable and avoids shell escaping issues. +- Only use shell heredocs when you need to append to a file or when the content depends on shell variable expansion. +- If you must use a heredoc, always use the full syntax: \`cat << 'EOF' > filename\` — never omit \`cat\`.`; +} + +function nonInteractiveProcessGuidance(): string { + return ` +## Background Services (Non-Interactive) +- To start a service that must persist across turns, use: \`nohup > /tmp/.log 2>&1 &\` +- After starting, verify the process is running: + 1. \`echo $!\` to capture the PID immediately after starting + 2. \`sleep 1 && kill -0 \` to confirm it's still alive + 3. Check the log file for startup errors: \`cat /tmp/.log\` +- If a service fails to start, check the log file before retrying. +- For services that need time to initialize (databases, web servers), add a brief \`sleep 2\` and verify the port is open before proceeding.`; +} + +function nonInteractiveErrorRecovery(): string { + return ` +## Error Recovery (Non-Interactive) +- **Don't blindly retry:** When a tool call fails, analyze the error before retrying. Do not immediately retry with the same arguments. +- **Web fetch failures:** If web_fetch fails, try simplifying the prompt or use google_web_search as an alternative to find the information. +- **Shell failures:** Check error codes and run diagnostic commands before retrying. For compilation errors, fix one issue at a time rather than attempting multiple fixes simultaneously. +- **Maximum retries:** Attempt the same approach at most 2 times. If it fails twice, try an alternative strategy or tool. +- **Avoid loops:** If you find yourself repeating the same sequence of actions, stop and reassess your approach. + +## Missing Commands / "command not found" +- When a command is not found, install the package that provides it before retrying. +- Use \`apt-get install -y \` (Debian/Ubuntu) or \`yum install -y \` (RHEL/CentOS). +- Common mappings: + - pgrep, ps, pidof → procps + - ss → iproute2 + - netstat, ifconfig → net-tools + - curl → curl + - jq → jq + - dig, nslookup → dnsutils (or bind-utils on RHEL) + - ip → iproute2 + - lsof → lsof + - wget → wget + - tree → tree + - zip/unzip → zip / unzip +- If you don't know the package name, try \`apt-cache search \` or \`yum provides \`. + +## Process Verification Without pgrep +- If pgrep/ps are unavailable and you can't install them, verify processes using: + - \`ls /proc/[PID]\` to check if a PID is still running + - \`kill -0 \` to test if a process exists (returns 0 if it does) + - \`cat /proc//cmdline\` to inspect what a process is running`; +} + function formatToolName(name: string): string { return `\`${name}\``; } diff --git a/packages/core/src/scheduler/state-manager.ts b/packages/core/src/scheduler/state-manager.ts index 42da1d78ac..fb16125340 100644 --- a/packages/core/src/scheduler/state-manager.ts +++ b/packages/core/src/scheduler/state-manager.ts @@ -17,8 +17,7 @@ import type { ExecutingToolCall, ToolCallResponseInfo, } from './types.js'; -import { CoreToolCallStatus } from './types.js'; -import { ROOT_SCHEDULER_ID } from './types.js'; +import { CoreToolCallStatus, ROOT_SCHEDULER_ID } from './types.js'; import type { ToolConfirmationOutcome, ToolResultDisplay, diff --git a/packages/core/src/scheduler/tool-executor.test.ts b/packages/core/src/scheduler/tool-executor.test.ts index 53b244031d..1cbee019c6 100644 --- a/packages/core/src/scheduler/tool-executor.test.ts +++ b/packages/core/src/scheduler/tool-executor.test.ts @@ -6,13 +6,12 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { ToolExecutor } from './tool-executor.js'; -import type { Config } from '../index.js'; +import type { Config, AnyToolInvocation } from '../index.js'; import type { ToolResult } from '../tools/tools.js'; import { makeFakeConfig } from '../test-utils/config.js'; import { MockTool } from '../test-utils/mock-tool.js'; import type { ScheduledToolCall } from './types.js'; import { CoreToolCallStatus } from './types.js'; -import type { AnyToolInvocation } from '../index.js'; import { SHELL_TOOL_NAME } from '../tools/tool-names.js'; import * as fileUtils from '../utils/fileUtils.js'; import * as coreToolHookTriggers from '../core/coreToolHookTriggers.js'; diff --git a/packages/core/src/services/chatCompressionService.test.ts b/packages/core/src/services/chatCompressionService.test.ts index 4ddd38e25c..855da90068 100644 --- a/packages/core/src/services/chatCompressionService.test.ts +++ b/packages/core/src/services/chatCompressionService.test.ts @@ -718,8 +718,9 @@ describe('ChatCompressionService', () => { it('should use high-fidelity original history for summarization when under the limit, but truncated version for active window', async () => { // Large response in the "to compress" section (first message) - // 300,000 chars is ~75k tokens, well under the 1,000,000 summarizer limit. - const massiveText = 'a'.repeat(300000); + // 500,000 chars is ~125k tokens, well under the 1,000,000 summarizer limit + // but exceeds COMPRESSION_FUNCTION_RESPONSE_TOKEN_BUDGET (75k). + const massiveText = 'a'.repeat(500000); const history: Content[] = [ { role: 'user', diff --git a/packages/core/src/services/chatCompressionService.ts b/packages/core/src/services/chatCompressionService.ts index 432c08dd1e..272d4c0939 100644 --- a/packages/core/src/services/chatCompressionService.ts +++ b/packages/core/src/services/chatCompressionService.ts @@ -12,7 +12,7 @@ import { tokenLimit } from '../core/tokenLimits.js'; import { getCompressionPrompt } from '../core/prompts.js'; import { getResponseText } from '../utils/partUtils.js'; import { logChatCompression } from '../telemetry/loggers.js'; -import { makeChatCompressionEvent } from '../telemetry/types.js'; +import { makeChatCompressionEvent, LlmRole } from '../telemetry/types.js'; import { saveTruncatedToolOutput, formatTruncatedToolOutput, @@ -32,7 +32,6 @@ import { PREVIEW_GEMINI_3_1_MODEL, } from '../config/models.js'; import { PreCompressTrigger } from '../hooks/types.js'; -import { LlmRole } from '../telemetry/types.js'; /** * Default threshold for compression token count as a fraction of the model's @@ -44,12 +43,12 @@ const DEFAULT_COMPRESSION_TOKEN_THRESHOLD = 0.5; * The fraction of the latest chat history to keep. A value of 0.3 * means that only the last 30% of the chat history will be kept after compression. */ -const COMPRESSION_PRESERVE_THRESHOLD = 0.3; +const COMPRESSION_PRESERVE_THRESHOLD = 0.4; /** * The budget for function response tokens in the preserved history. */ -const COMPRESSION_FUNCTION_RESPONSE_TOKEN_BUDGET = 50_000; +const COMPRESSION_FUNCTION_RESPONSE_TOKEN_BUDGET = 75_000; /** * Returns the index of the oldest item to keep when compressing. May return diff --git a/packages/core/src/services/chatRecordingService.test.ts b/packages/core/src/services/chatRecordingService.test.ts index 61ba3d32a3..086a7b6ff5 100644 --- a/packages/core/src/services/chatRecordingService.test.ts +++ b/packages/core/src/services/chatRecordingService.test.ts @@ -86,6 +86,21 @@ describe('ChatRecordingService', () => { expect(files[0]).toMatch(/^session-.*-test-ses\.json$/); }); + it('should include the conversation kind when specified', () => { + chatRecordingService.initialize(undefined, 'subagent'); + chatRecordingService.recordMessage({ + type: 'user', + content: 'ping', + model: 'm', + }); + + const sessionFile = chatRecordingService.getConversationFilePath()!; + const conversation = JSON.parse( + fs.readFileSync(sessionFile, 'utf8'), + ) as ConversationRecord; + expect(conversation.kind).toBe('subagent'); + }); + it('should resume from an existing session if provided', () => { const chatsDir = path.join(testTempDir, 'chats'); fs.mkdirSync(chatsDir, { recursive: true }); diff --git a/packages/core/src/services/chatRecordingService.ts b/packages/core/src/services/chatRecordingService.ts index 0b94825353..2afbd16657 100644 --- a/packages/core/src/services/chatRecordingService.ts +++ b/packages/core/src/services/chatRecordingService.ts @@ -102,6 +102,8 @@ export interface ConversationRecord { summary?: string; /** Workspace directories added during the session via /dir add */ directories?: string[]; + /** The kind of conversation (main agent or subagent) */ + kind?: 'main' | 'subagent'; } /** @@ -128,6 +130,7 @@ export class ChatRecordingService { private cachedLastConvData: string | null = null; private sessionId: string; private projectHash: string; + private kind?: 'main' | 'subagent'; private queuedThoughts: Array = []; private queuedTokens: TokensSummary | null = null; private config: Config; @@ -141,13 +144,21 @@ export class ChatRecordingService { /** * Initializes the chat recording service: creates a new conversation file and associates it with * this service instance, or resumes from an existing session if resumedSessionData is provided. + * + * @param resumedSessionData Data from a previous session to resume from. + * @param kind The kind of conversation (main or subagent). */ - initialize(resumedSessionData?: ResumedSessionData): void { + initialize( + resumedSessionData?: ResumedSessionData, + kind?: 'main' | 'subagent', + ): void { try { + this.kind = kind; if (resumedSessionData) { // Resume from existing session this.conversationFile = resumedSessionData.filePath; this.sessionId = resumedSessionData.conversation.sessionId; + this.kind = resumedSessionData.conversation.kind; // Update the session ID in the existing file this.updateConversation((conversation) => { @@ -180,6 +191,7 @@ export class ChatRecordingService { startTime: new Date().toISOString(), lastUpdated: new Date().toISOString(), messages: [], + kind: this.kind, }); } @@ -419,6 +431,7 @@ export class ChatRecordingService { private readConversation(): ConversationRecord { try { this.cachedLastConvData = fs.readFileSync(this.conversationFile!, 'utf8'); + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return JSON.parse(this.cachedLastConvData); } catch (error) { // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion @@ -434,6 +447,7 @@ export class ChatRecordingService { startTime: new Date().toISOString(), lastUpdated: new Date().toISOString(), messages: [], + kind: this.kind, }; } } diff --git a/packages/core/src/services/loopDetectionService.test.ts b/packages/core/src/services/loopDetectionService.test.ts index 4bfd0df099..fb6724bd87 100644 --- a/packages/core/src/services/loopDetectionService.test.ts +++ b/packages/core/src/services/loopDetectionService.test.ts @@ -26,7 +26,7 @@ vi.mock('../telemetry/loggers.js', () => ({ logLlmLoopCheck: vi.fn(), })); -const TOOL_CALL_LOOP_THRESHOLD = 5; +const TOOL_CALL_LOOP_THRESHOLD = 4; const CONTENT_LOOP_THRESHOLD = 10; const CONTENT_CHUNK_SIZE = 50; @@ -806,15 +806,15 @@ describe('LoopDetectionService LLM Checks', () => { }; it('should not trigger LLM check before LLM_CHECK_AFTER_TURNS', async () => { - await advanceTurns(29); + await advanceTurns(19); expect(mockBaseLlmClient.generateJson).not.toHaveBeenCalled(); }); - it('should trigger LLM check on the 30th turn', async () => { + it('should trigger LLM check on the 20th turn', async () => { mockBaseLlmClient.generateJson = vi .fn() .mockResolvedValue({ unproductive_state_confidence: 0.1 }); - await advanceTurns(30); + await advanceTurns(20); expect(mockBaseLlmClient.generateJson).toHaveBeenCalledTimes(1); expect(mockBaseLlmClient.generateJson).toHaveBeenCalledWith( expect.objectContaining({ @@ -828,12 +828,12 @@ describe('LoopDetectionService LLM Checks', () => { }); it('should detect a cognitive loop when confidence is high', async () => { - // First check at turn 30 + // First check at turn 20 mockBaseLlmClient.generateJson = vi.fn().mockResolvedValue({ unproductive_state_confidence: 0.85, unproductive_state_analysis: 'Repetitive actions', }); - await advanceTurns(30); + await advanceTurns(20); expect(mockBaseLlmClient.generateJson).toHaveBeenCalledTimes(1); expect(mockBaseLlmClient.generateJson).toHaveBeenCalledWith( expect.objectContaining({ @@ -843,13 +843,13 @@ describe('LoopDetectionService LLM Checks', () => { // The confidence of 0.85 will result in a low interval. // The interval will be: 5 + (15 - 5) * (1 - 0.85) = 5 + 10 * 0.15 = 6.5 -> rounded to 7 - await advanceTurns(6); // advance to turn 36 + await advanceTurns(6); // advance to turn 26 mockBaseLlmClient.generateJson = vi.fn().mockResolvedValue({ unproductive_state_confidence: 0.95, unproductive_state_analysis: 'Repetitive actions', }); - const finalResult = await service.turnStarted(abortController.signal); // This is turn 37 + const finalResult = await service.turnStarted(abortController.signal); // This is turn 27 expect(finalResult).toBe(true); expect(loggers.logLoopDetected).toHaveBeenCalledWith( @@ -867,7 +867,7 @@ describe('LoopDetectionService LLM Checks', () => { unproductive_state_confidence: 0.5, unproductive_state_analysis: 'Looks okay', }); - await advanceTurns(30); + await advanceTurns(20); const result = await service.turnStarted(abortController.signal); expect(result).toBe(false); expect(loggers.logLoopDetected).not.toHaveBeenCalled(); @@ -878,13 +878,13 @@ describe('LoopDetectionService LLM Checks', () => { mockBaseLlmClient.generateJson = vi .fn() .mockResolvedValue({ unproductive_state_confidence: 0.0 }); - await advanceTurns(30); // First check at turn 30 + await advanceTurns(20); // First check at turn 20 expect(mockBaseLlmClient.generateJson).toHaveBeenCalledTimes(1); - await advanceTurns(14); // Advance to turn 44 + await advanceTurns(14); // Advance to turn 34 expect(mockBaseLlmClient.generateJson).toHaveBeenCalledTimes(1); - await service.turnStarted(abortController.signal); // Turn 45 + await service.turnStarted(abortController.signal); // Turn 35 expect(mockBaseLlmClient.generateJson).toHaveBeenCalledTimes(2); }); @@ -892,7 +892,7 @@ describe('LoopDetectionService LLM Checks', () => { mockBaseLlmClient.generateJson = vi .fn() .mockRejectedValue(new Error('API error')); - await advanceTurns(30); + await advanceTurns(20); const result = await service.turnStarted(abortController.signal); expect(result).toBe(false); expect(loggers.logLoopDetected).not.toHaveBeenCalled(); @@ -901,7 +901,7 @@ describe('LoopDetectionService LLM Checks', () => { it('should not trigger LLM check when disabled for session', async () => { service.disableForSession(); expect(loggers.logLoopDetectionDisabled).toHaveBeenCalledTimes(1); - await advanceTurns(30); + await advanceTurns(20); const result = await service.turnStarted(abortController.signal); expect(result).toBe(false); expect(mockBaseLlmClient.generateJson).not.toHaveBeenCalled(); @@ -924,7 +924,7 @@ describe('LoopDetectionService LLM Checks', () => { .fn() .mockResolvedValue({ unproductive_state_confidence: 0.1 }); - await advanceTurns(30); + await advanceTurns(20); expect(mockBaseLlmClient.generateJson).toHaveBeenCalledTimes(1); const calledArg = vi.mocked(mockBaseLlmClient.generateJson).mock @@ -949,7 +949,7 @@ describe('LoopDetectionService LLM Checks', () => { unproductive_state_analysis: 'Main says loop', }); - await advanceTurns(30); + await advanceTurns(20); // It should have called generateJson twice expect(mockBaseLlmClient.generateJson).toHaveBeenCalledTimes(2); @@ -989,7 +989,7 @@ describe('LoopDetectionService LLM Checks', () => { unproductive_state_analysis: 'Main says no loop', }); - await advanceTurns(30); + await advanceTurns(20); expect(mockBaseLlmClient.generateJson).toHaveBeenCalledTimes(2); expect(mockBaseLlmClient.generateJson).toHaveBeenNthCalledWith( @@ -1032,7 +1032,7 @@ describe('LoopDetectionService LLM Checks', () => { unproductive_state_analysis: 'Flash says loop', }); - await advanceTurns(30); + await advanceTurns(20); // It should have called generateJson only once expect(mockBaseLlmClient.generateJson).toHaveBeenCalledTimes(1); diff --git a/packages/core/src/services/loopDetectionService.ts b/packages/core/src/services/loopDetectionService.ts index 8ae2b77898..2c08905ee1 100644 --- a/packages/core/src/services/loopDetectionService.ts +++ b/packages/core/src/services/loopDetectionService.ts @@ -18,6 +18,7 @@ import { LoopDetectionDisabledEvent, LoopType, LlmLoopCheckEvent, + LlmRole, } from '../telemetry/types.js'; import type { Config } from '../config/config.js'; import { @@ -25,9 +26,8 @@ import { isFunctionResponse, } from '../utils/messageInspectors.js'; import { debugLogger } from '../utils/debugLogger.js'; -import { LlmRole } from '../telemetry/types.js'; -const TOOL_CALL_LOOP_THRESHOLD = 5; +const TOOL_CALL_LOOP_THRESHOLD = 4; const CONTENT_LOOP_THRESHOLD = 10; const CONTENT_CHUNK_SIZE = 50; const MAX_HISTORY_LENGTH = 5000; @@ -40,7 +40,7 @@ const LLM_LOOP_CHECK_HISTORY_COUNT = 20; /** * The number of turns that must pass in a single prompt before the LLM-based loop check is activated. */ -const LLM_CHECK_AFTER_TURNS = 30; +const LLM_CHECK_AFTER_TURNS = 20; /** * The default interval, in number of turns, at which the LLM-based loop check is performed. @@ -105,6 +105,7 @@ export class LoopDetectionService { // Tool call tracking private lastToolCallKey: string | null = null; private toolCallRepetitionCount: number = 0; + private recentToolCallKeys: string[] = []; // Content streaming tracking private streamContentHistory = ''; @@ -217,6 +218,53 @@ export class LoopDetectionService { ); return true; } + + // Alternating pattern detection: track last 12 tool calls and detect + // when a pattern of 2-3 distinct calls repeats 3+ times. + this.recentToolCallKeys.push(key); + if (this.recentToolCallKeys.length > 12) { + this.recentToolCallKeys.shift(); + } + if (this.detectAlternatingPattern()) { + logLoopDetected( + this.config, + new LoopDetectedEvent( + LoopType.CONSECUTIVE_IDENTICAL_TOOL_CALLS, + this.promptId, + ), + ); + return true; + } + + return false; + } + + /** + * Detects alternating patterns like A->B->A->B->A->B or A->B->C->A->B->C. + * Checks if a pattern of length 2 or 3 repeats at least 3 times at the + * end of the recent tool call history. + */ + private detectAlternatingPattern(): boolean { + const keys = this.recentToolCallKeys; + // Check patterns of length 2 and 3 + for (const patternLen of [2, 3]) { + const minRequired = patternLen * 3; // Need at least 3 repetitions + if (keys.length < minRequired) continue; + + const pattern = keys.slice(keys.length - patternLen); + let repetitions = 1; + for (let i = keys.length - patternLen * 2; i >= 0; i -= patternLen) { + const segment = keys.slice(i, i + patternLen); + if (segment.every((k, idx) => k === pattern[idx])) { + repetitions++; + } else { + break; + } + } + if (repetitions >= 3) { + return true; + } + } return false; } @@ -613,6 +661,7 @@ export class LoopDetectionService { private resetToolCallCount(): void { this.lastToolCallKey = null; this.toolCallRepetitionCount = 0; + this.recentToolCallKeys = []; } private resetContentTracking(resetHistory = true): void { diff --git a/packages/core/src/skills/skillLoader.ts b/packages/core/src/skills/skillLoader.ts index 08374ec93a..e746caa179 100644 --- a/packages/core/src/skills/skillLoader.ts +++ b/packages/core/src/skills/skillLoader.ts @@ -7,7 +7,7 @@ import * as fs from 'node:fs/promises'; import * as path from 'node:path'; import { glob } from 'glob'; -import yaml from 'js-yaml'; +import { load } from 'js-yaml'; import { debugLogger } from '../utils/debugLogger.js'; import { coreEvents } from '../utils/events.js'; @@ -40,7 +40,7 @@ function parseFrontmatter( content: string, ): { name: string; description: string } | null { try { - const parsed = yaml.load(content); + const parsed = load(content); if (parsed && typeof parsed === 'object') { // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion const { name, description } = parsed as Record; diff --git a/packages/core/src/telemetry/clearcut-logger/clearcut-logger.test.ts b/packages/core/src/telemetry/clearcut-logger/clearcut-logger.test.ts index c5a00bc11d..ed87bb34fc 100644 --- a/packages/core/src/telemetry/clearcut-logger/clearcut-logger.test.ts +++ b/packages/core/src/telemetry/clearcut-logger/clearcut-logger.test.ts @@ -4,7 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -import 'vitest'; import { vi, describe, diff --git a/packages/core/src/telemetry/gcp-exporters.ts b/packages/core/src/telemetry/gcp-exporters.ts index 528b15b22e..c7429383eb 100644 --- a/packages/core/src/telemetry/gcp-exporters.ts +++ b/packages/core/src/telemetry/gcp-exporters.ts @@ -9,9 +9,8 @@ import { TraceExporter } from '@google-cloud/opentelemetry-cloud-trace-exporter' import { MetricExporter } from '@google-cloud/opentelemetry-cloud-monitoring-exporter'; import { Logging } from '@google-cloud/logging'; import type { Log } from '@google-cloud/logging'; -import { hrTimeToMilliseconds } from '@opentelemetry/core'; +import { hrTimeToMilliseconds, ExportResultCode } from '@opentelemetry/core'; import type { ExportResult } from '@opentelemetry/core'; -import { ExportResultCode } from '@opentelemetry/core'; import type { ReadableLogRecord, LogRecordExporter, diff --git a/packages/core/src/telemetry/loggers.test.ts b/packages/core/src/telemetry/loggers.test.ts index 316cf0b33f..6a0f6f3240 100644 --- a/packages/core/src/telemetry/loggers.test.ts +++ b/packages/core/src/telemetry/loggers.test.ts @@ -10,6 +10,7 @@ import type { CompletedToolCall, ContentGeneratorConfig, ErroredToolCall, + MessageBus, } from '../index.js'; import { CoreToolCallStatus, @@ -19,11 +20,10 @@ import { ToolConfirmationOutcome, ToolErrorType, ToolRegistry, - type MessageBus, } from '../index.js'; import { OutputFormat } from '../output/types.js'; import { logs } from '@opentelemetry/api-logs'; -import type { Config } from '../config/config.js'; +import type { Config, GeminiCLIExtension } from '../config/config.js'; import { logApiError, logApiRequest, @@ -100,7 +100,6 @@ import { FileOperation } from './metrics.js'; import * as sdk from './sdk.js'; import { createMockMessageBus } from '../test-utils/mock-message-bus.js'; import { vi, describe, beforeEach, it, expect, afterEach } from 'vitest'; -import { type GeminiCLIExtension } from '../config/config.js'; import { FinishReason, type CallableTool, @@ -1107,6 +1106,7 @@ describe('loggers', () => { new ToolRegistry(cfg1, {} as unknown as MessageBus), getUserMemory: () => 'user-memory', + isInteractive: () => false, } as unknown as Config; const mockGeminiClient = new GeminiClient(cfg2); diff --git a/packages/core/src/telemetry/uiTelemetry.test.ts b/packages/core/src/telemetry/uiTelemetry.test.ts index 52f0911730..d1a3b1a9a6 100644 --- a/packages/core/src/telemetry/uiTelemetry.test.ts +++ b/packages/core/src/telemetry/uiTelemetry.test.ts @@ -8,8 +8,8 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { UiTelemetryService } from './uiTelemetry.js'; import { ToolCallDecision } from './tool-call-decision.js'; import type { ApiErrorEvent, ApiResponseEvent } from './types.js'; -import { ToolCallEvent } from './types.js'; import { + ToolCallEvent, EVENT_API_ERROR, EVENT_API_RESPONSE, EVENT_TOOL_CALL, diff --git a/packages/core/src/telemetry/uiTelemetry.ts b/packages/core/src/telemetry/uiTelemetry.ts index 8c9f2adb83..669b6a8c68 100644 --- a/packages/core/src/telemetry/uiTelemetry.ts +++ b/packages/core/src/telemetry/uiTelemetry.ts @@ -16,10 +16,9 @@ import type { ApiErrorEvent, ApiResponseEvent, ToolCallEvent, + LlmRole, } from './types.js'; -import type { LlmRole } from './types.js'; - export type UiEvent = | (ApiResponseEvent & { 'event.name': typeof EVENT_API_RESPONSE }) | (ApiErrorEvent & { 'event.name': typeof EVENT_API_ERROR }) diff --git a/packages/core/src/tools/definitions/__snapshots__/coreToolsModelSnapshots.test.ts.snap b/packages/core/src/tools/definitions/__snapshots__/coreToolsModelSnapshots.test.ts.snap index 7e3e1dcf80..8ec768d843 100644 --- a/packages/core/src/tools/definitions/__snapshots__/coreToolsModelSnapshots.test.ts.snap +++ b/packages/core/src/tools/definitions/__snapshots__/coreToolsModelSnapshots.test.ts.snap @@ -547,7 +547,7 @@ A good instruction should concisely answer: "type": "string", }, "new_string": { - "description": "The exact literal text to replace \`old_string\` with, preferably unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic.", + "description": "The exact literal text to replace \`old_string\` with, preferably unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic. Do not use omission placeholders like '(rest of methods ...)', '...', or 'unchanged code'; provide exact literal code.", "type": "string", }, "old_string": { @@ -665,7 +665,7 @@ exports[`coreTools snapshots for specific models > Model: gemini-2.5-pro > snaps "parametersJsonSchema": { "properties": { "content": { - "description": "The content to write to the file.", + "description": "The content to write to the file. Do not use omission placeholders like '(rest of methods ...)', '...', or 'unchanged code'; provide complete literal content.", "type": "string", }, "file_path": { @@ -1312,7 +1312,7 @@ The user has the ability to modify the \`new_string\` content. If modified, this "type": "string", }, "new_string": { - "description": "The exact literal text to replace \`old_string\` with, unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic.", + "description": "The exact literal text to replace \`old_string\` with, unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic. Do not use omission placeholders like '(rest of methods ...)', '...', or 'unchanged code'; provide exact literal code.", "type": "string", }, "old_string": { @@ -1429,7 +1429,7 @@ The user has the ability to modify \`content\`. If modified, this will be stated "parametersJsonSchema": { "properties": { "content": { - "description": "The content to write to the file.", + "description": "The content to write to the file. Do not use omission placeholders like '(rest of methods ...)', '...', or 'unchanged code'; provide complete literal content.", "type": "string", }, "file_path": { diff --git a/packages/core/src/tools/definitions/model-family-sets/default-legacy.ts b/packages/core/src/tools/definitions/model-family-sets/default-legacy.ts index fad72047a9..ae9c4831ad 100644 --- a/packages/core/src/tools/definitions/model-family-sets/default-legacy.ts +++ b/packages/core/src/tools/definitions/model-family-sets/default-legacy.ts @@ -71,7 +71,8 @@ export const DEFAULT_LEGACY_SET: CoreToolSet = { type: 'string', }, content: { - description: 'The content to write to the file.', + description: + "The content to write to the file. Do not use omission placeholders like '(rest of methods ...)', '...', or 'unchanged code'; provide complete literal content.", type: 'string', }, }, @@ -332,7 +333,7 @@ A good instruction should concisely answer: }, new_string: { description: - 'The exact literal text to replace `old_string` with, preferably unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic.', + "The exact literal text to replace `old_string` with, preferably unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic. Do not use omission placeholders like '(rest of methods ...)', '...', or 'unchanged code'; provide exact literal code.", type: 'string', }, expected_replacements: { diff --git a/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts b/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts index 1ceca46d9f..6bb2809874 100644 --- a/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts +++ b/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts @@ -73,7 +73,8 @@ The user has the ability to modify \`content\`. If modified, this will be stated type: 'string', }, content: { - description: 'The content to write to the file.', + description: + "The content to write to the file. Do not use omission placeholders like '(rest of methods ...)', '...', or 'unchanged code'; provide complete literal content.", type: 'string', }, }, @@ -310,7 +311,7 @@ The user has the ability to modify the \`new_string\` content. If modified, this }, new_string: { description: - 'The exact literal text to replace `old_string` with, unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic.', + "The exact literal text to replace `old_string` with, unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic. Do not use omission placeholders like '(rest of methods ...)', '...', or 'unchanged code'; provide exact literal code.", type: 'string', }, expected_replacements: { diff --git a/packages/core/src/tools/edit.test.ts b/packages/core/src/tools/edit.test.ts index 3b8cbe9645..9c67515f38 100644 --- a/packages/core/src/tools/edit.test.ts +++ b/packages/core/src/tools/edit.test.ts @@ -607,6 +607,53 @@ function doIt() { }; expect(tool.validateToolParams(params)).toMatch(/Path not in workspace/); }); + + it('should reject omission placeholder in new_string when old_string does not contain that placeholder', () => { + const params: EditToolParams = { + file_path: path.join(rootDir, 'test.txt'), + instruction: 'An instruction', + old_string: 'old content', + new_string: '(rest of methods ...)', + }; + expect(tool.validateToolParams(params)).toBe( + "`new_string` contains an omission placeholder (for example 'rest of methods ...'). Provide exact literal replacement text.", + ); + }); + + it('should reject new_string when it contains an additional placeholder not present in old_string', () => { + const params: EditToolParams = { + file_path: path.join(rootDir, 'test.txt'), + instruction: 'An instruction', + old_string: '(rest of methods ...)', + new_string: `(rest of methods ...) +(unchanged code ...)`, + }; + expect(tool.validateToolParams(params)).toBe( + "`new_string` contains an omission placeholder (for example 'rest of methods ...'). Provide exact literal replacement text.", + ); + }); + + it('should allow omission placeholders when all are already present in old_string', () => { + const params: EditToolParams = { + file_path: path.join(rootDir, 'test.txt'), + instruction: 'An instruction', + old_string: `(rest of methods ...) +(unchanged code ...)`, + new_string: `(unchanged code ...) +(rest of methods ...)`, + }; + expect(tool.validateToolParams(params)).toBeNull(); + }); + + it('should allow normal code that contains placeholder text in a string literal', () => { + const params: EditToolParams = { + file_path: path.join(rootDir, 'test.ts'), + instruction: 'Update string literal', + old_string: 'const msg = "old";', + new_string: 'const msg = "(rest of methods ...)";', + }; + expect(tool.validateToolParams(params)).toBeNull(); + }); }); describe('execute', () => { diff --git a/packages/core/src/tools/edit.ts b/packages/core/src/tools/edit.ts index 3df9c21b5e..edd6959949 100644 --- a/packages/core/src/tools/edit.ts +++ b/packages/core/src/tools/edit.ts @@ -38,10 +38,11 @@ import { import { IdeClient } from '../ide/ide-client.js'; import { FixLLMEditWithInstruction } from '../utils/llm-edit-fixer.js'; import { safeLiteralReplace, detectLineEnding } from '../utils/textUtils.js'; -import { EditStrategyEvent } from '../telemetry/types.js'; -import { logEditStrategy } from '../telemetry/loggers.js'; -import { EditCorrectionEvent } from '../telemetry/types.js'; -import { logEditCorrectionEvent } from '../telemetry/loggers.js'; +import { EditStrategyEvent, EditCorrectionEvent } from '../telemetry/types.js'; +import { + logEditStrategy, + logEditCorrectionEvent, +} from '../telemetry/loggers.js'; import { correctPath } from '../utils/pathCorrector.js'; import { @@ -53,6 +54,7 @@ import { debugLogger } from '../utils/debugLogger.js'; import levenshtein from 'fast-levenshtein'; import { EDIT_DEFINITION } from './definitions/coreTools.js'; import { resolveToolDeclaration } from './definitions/resolver.js'; +import { detectOmissionPlaceholders } from './omissionPlaceholderDetector.js'; const ENABLE_FUZZY_MATCH_RECOVERY = true; const FUZZY_MATCH_THRESHOLD = 0.1; // Allow up to 10% weighted difference @@ -136,6 +138,17 @@ async function calculateExactReplacement( const normalizedReplace = new_string.replace(/\r\n/g, '\n'); const exactOccurrences = normalizedCode.split(normalizedSearch).length - 1; + const expectedReplacements = params.expected_replacements ?? 1; + + if (exactOccurrences > expectedReplacements) { + return { + newContent: currentContent, + occurrences: exactOccurrences, + finalOldString: normalizedSearch, + finalNewString: normalizedReplace, + }; + } + if (exactOccurrences > 0) { let modifiedCode = safeLiteralReplace( normalizedCode, @@ -972,6 +985,19 @@ export class EditTool } params.file_path = filePath; + const newPlaceholders = detectOmissionPlaceholders(params.new_string); + if (newPlaceholders.length > 0) { + const oldPlaceholders = new Set( + detectOmissionPlaceholders(params.old_string), + ); + + for (const placeholder of newPlaceholders) { + if (!oldPlaceholders.has(placeholder)) { + return "`new_string` contains an omission placeholder (for example 'rest of methods ...'). Provide exact literal replacement text."; + } + } + } + return this.config.validatePathAccess(params.file_path); } diff --git a/packages/core/src/tools/mcp-tool.ts b/packages/core/src/tools/mcp-tool.ts index 1279d0f705..6faa30c673 100644 --- a/packages/core/src/tools/mcp-tool.ts +++ b/packages/core/src/tools/mcp-tool.ts @@ -10,13 +10,13 @@ import type { ToolInvocation, ToolMcpConfirmationDetails, ToolResult, + PolicyUpdateOptions, } from './tools.js'; import { BaseDeclarativeTool, BaseToolInvocation, Kind, ToolConfirmationOutcome, - type PolicyUpdateOptions, } from './tools.js'; import type { CallableTool, FunctionCall, Part } from '@google/genai'; import { ToolErrorType } from './tool-error.js'; diff --git a/packages/core/src/tools/omissionPlaceholderDetector.test.ts b/packages/core/src/tools/omissionPlaceholderDetector.test.ts new file mode 100644 index 0000000000..4e574d5e22 --- /dev/null +++ b/packages/core/src/tools/omissionPlaceholderDetector.test.ts @@ -0,0 +1,63 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { detectOmissionPlaceholders } from './omissionPlaceholderDetector.js'; + +describe('detectOmissionPlaceholders', () => { + it('detects standalone placeholder lines', () => { + expect(detectOmissionPlaceholders('(rest of methods ...)')).toEqual([ + 'rest of methods ...', + ]); + expect(detectOmissionPlaceholders('(rest of code ...)')).toEqual([ + 'rest of code ...', + ]); + expect(detectOmissionPlaceholders('(unchanged code ...)')).toEqual([ + 'unchanged code ...', + ]); + expect(detectOmissionPlaceholders('// rest of methods ...')).toEqual([ + 'rest of methods ...', + ]); + }); + + it('detects case-insensitive placeholders', () => { + expect(detectOmissionPlaceholders('(Rest Of Methods ...)')).toEqual([ + 'rest of methods ...', + ]); + }); + + it('detects multiple placeholder lines in one input', () => { + const text = `class Example { + run() {} + (rest of methods ...) + (unchanged code ...) +}`; + expect(detectOmissionPlaceholders(text)).toEqual([ + 'rest of methods ...', + 'unchanged code ...', + ]); + }); + + it('does not detect placeholders embedded in normal code', () => { + expect( + detectOmissionPlaceholders( + 'const note = "(rest of methods ...)";\nconsole.log(note);', + ), + ).toEqual([]); + }); + + it('does not detect omission phrase when inline in a comment', () => { + expect( + detectOmissionPlaceholders('return value; // rest of methods ...'), + ).toEqual([]); + }); + + it('does not detect unrelated ellipsis text', () => { + expect(detectOmissionPlaceholders('const message = "loading...";')).toEqual( + [], + ); + }); +}); diff --git a/packages/core/src/tools/omissionPlaceholderDetector.ts b/packages/core/src/tools/omissionPlaceholderDetector.ts new file mode 100644 index 0000000000..7057a7f09d --- /dev/null +++ b/packages/core/src/tools/omissionPlaceholderDetector.ts @@ -0,0 +1,106 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +const OMITTED_PREFIXES = new Set([ + 'rest of', + 'rest of method', + 'rest of methods', + 'rest of code', + 'unchanged code', + 'unchanged method', + 'unchanged methods', +]); + +function isAllDots(str: string): boolean { + if (str.length === 0) { + return false; + } + for (let i = 0; i < str.length; i++) { + if (str[i] !== '.') { + return false; + } + } + return true; +} + +function normalizeWhitespace(input: string): string { + const segments: string[] = []; + let current = ''; + + for (const char of input) { + if (char === ' ' || char === '\t' || char === '\n' || char === '\r') { + if (current.length > 0) { + segments.push(current); + current = ''; + } + continue; + } + current += char; + } + + if (current.length > 0) { + segments.push(current); + } + + return segments.join(' '); +} + +function normalizePlaceholder(line: string): string | null { + let text = line.trim(); + if (!text) { + return null; + } + + if (text.startsWith('//')) { + text = text.slice(2).trim(); + } + + if (text.startsWith('(') && text.endsWith(')')) { + text = text.slice(1, -1).trim(); + } + + const ellipsisStart = text.indexOf('...'); + if (ellipsisStart < 0) { + return null; + } + + const prefixRaw = text.slice(0, ellipsisStart).trim().toLowerCase(); + const suffixRaw = text.slice(ellipsisStart + 3).trim(); + const prefix = normalizeWhitespace(prefixRaw); + + if (!OMITTED_PREFIXES.has(prefix)) { + return null; + } + + if (suffixRaw.length > 0 && !isAllDots(suffixRaw)) { + return null; + } + + return `${prefix} ...`; +} + +/** + * Detects shorthand omission placeholders such as: + * - (rest of methods ...) + * - (rest of code ...) + * - (unchanged code ...) + * - // rest of methods ... + * + * Returns all placeholders found as normalized tokens. + */ +export function detectOmissionPlaceholders(text: string): string[] { + const lines = text.replaceAll('\r\n', '\n').split('\n'); + const matches: string[] = []; + + for (const rawLine of lines) { + const normalized = normalizePlaceholder(rawLine); + if (normalized) { + matches.push(normalized); + } + } + + return matches; +} diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 5fc3ca7f25..907d117439 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -42,7 +42,7 @@ vi.mock('crypto'); vi.mock('../utils/summarizer.js'); import { initializeShellParsers } from '../utils/shell-utils.js'; -import { ShellTool } from './shell.js'; +import { ShellTool, OUTPUT_UPDATE_INTERVAL_MS } from './shell.js'; import { debugLogger } from '../index.js'; import { type Config } from '../config/config.js'; import { @@ -58,7 +58,6 @@ import * as crypto from 'node:crypto'; import * as summarizer from '../utils/summarizer.js'; import { ToolErrorType } from './tool-error.js'; import { ToolConfirmationOutcome } from './tools.js'; -import { OUTPUT_UPDATE_INTERVAL_MS } from './shell.js'; import { SHELL_TOOL_NAME } from './tool-names.js'; import { WorkspaceContext } from '../utils/workspaceContext.js'; import { diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 76db302f42..741272f555 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -16,13 +16,13 @@ import type { ToolResult, ToolCallConfirmationDetails, ToolExecuteConfirmationDetails, + PolicyUpdateOptions, } from './tools.js'; import { BaseDeclarativeTool, BaseToolInvocation, ToolConfirmationOutcome, Kind, - type PolicyUpdateOptions, } from './tools.js'; import { getErrorMessage } from '../utils/errors.js'; diff --git a/packages/core/src/tools/web-fetch.test.ts b/packages/core/src/tools/web-fetch.test.ts index 2e06a46ee5..6b7daebaae 100644 --- a/packages/core/src/tools/web-fetch.test.ts +++ b/packages/core/src/tools/web-fetch.test.ts @@ -141,7 +141,7 @@ describe('WebFetchTool', () => { setApprovalMode: vi.fn(), getProxy: vi.fn(), getGeminiClient: mockGetGeminiClient, - getRetryFetchErrors: vi.fn().mockReturnValue(false), + getRetryFetchErrors: vi.fn().mockReturnValue(true), modelConfigService: { getResolvedConfig: vi.fn().mockImplementation(({ model }) => ({ model, @@ -208,6 +208,8 @@ describe('WebFetchTool', () => { vi.spyOn(fetchUtils, 'fetchWithTimeout').mockRejectedValue( new Error('fetch failed'), ); + // Disable retries so test doesn't timeout waiting for backoff + vi.mocked(mockConfig.getRetryFetchErrors).mockReturnValue(false); const tool = new WebFetchTool(mockConfig, bus); const params = { prompt: 'fetch https://private.ip' }; const invocation = tool.build(params); diff --git a/packages/core/src/tools/web-fetch.ts b/packages/core/src/tools/web-fetch.ts index 9b6f832971..24460c23c0 100644 --- a/packages/core/src/tools/web-fetch.ts +++ b/packages/core/src/tools/web-fetch.ts @@ -8,13 +8,9 @@ import type { ToolCallConfirmationDetails, ToolInvocation, ToolResult, + ToolConfirmationOutcome, } from './tools.js'; -import { - BaseDeclarativeTool, - BaseToolInvocation, - Kind, - type ToolConfirmationOutcome, -} from './tools.js'; +import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import { ToolErrorType } from './tool-error.js'; import { getErrorMessage } from '../utils/errors.js'; @@ -35,8 +31,8 @@ import { WEB_FETCH_DEFINITION } from './definitions/coreTools.js'; import { resolveToolDeclaration } from './definitions/resolver.js'; import { LRUCache } from 'mnemonist'; -const URL_FETCH_TIMEOUT_MS = 10000; -const MAX_CONTENT_LENGTH = 100000; +const URL_FETCH_TIMEOUT_MS = 30000; +const MAX_CONTENT_LENGTH = 200000; // Rate limiting configuration const RATE_LIMIT_WINDOW_MS = 60000; // 1 minute @@ -160,67 +156,104 @@ class WebFetchToolInvocation extends BaseToolInvocation< super(params, messageBus, _toolName, _toolDisplayName); } - private async executeFallback(signal: AbortSignal): Promise { - const { validUrls: urls } = parsePrompt(this.params.prompt); - // For now, we only support one URL for fallback - let url = urls[0]; - + private async executeFallbackForUrl( + url: string, + signal: AbortSignal, + perUrlBudget: number, + ): Promise<{ content: string; error?: string }> { // Convert GitHub blob URL to raw URL - if (url.includes('github.com') && url.includes('/blob/')) { - url = url + let fetchUrl = url; + if (fetchUrl.includes('github.com') && fetchUrl.includes('/blob/')) { + fetchUrl = fetchUrl .replace('github.com', 'raw.githubusercontent.com') .replace('/blob/', '/'); } + const response = await retryWithBackoff( + async () => { + const res = await fetchWithTimeout(fetchUrl, URL_FETCH_TIMEOUT_MS); + if (!res.ok) { + const error = new Error( + `Request failed with status code ${res.status} ${res.statusText}`, + ); + (error as ErrorWithStatus).status = res.status; + throw error; + } + return res; + }, + { + retryFetchErrors: this.config.getRetryFetchErrors(), + signal, + }, + ); + + const rawContent = await response.text(); + const contentType = response.headers.get('content-type') || ''; + let textContent: string; + + // Only use html-to-text if content type is HTML, or if no content type is provided (assume HTML) + if (contentType.toLowerCase().includes('text/html') || contentType === '') { + textContent = convert(rawContent, { + wordwrap: false, + selectors: [ + { selector: 'a', options: { ignoreHref: true } }, + { selector: 'img', format: 'skip' }, + ], + }); + } else { + // For other content types (text/plain, application/json, etc.), use raw text + textContent = rawContent; + } + + // Per-URL content budget is the total budget divided by number of URLs + textContent = textContent.substring(0, perUrlBudget); + return { content: textContent }; + } + + private async executeFallback(signal: AbortSignal): Promise { + const { validUrls: urls } = parsePrompt(this.params.prompt); + try { - const response = await retryWithBackoff( - async () => { - const res = await fetchWithTimeout(url, URL_FETCH_TIMEOUT_MS); - if (!res.ok) { - const error = new Error( - `Request failed with status code ${res.status} ${res.statusText}`, - ); - (error as ErrorWithStatus).status = res.status; - throw error; - } - return res; - }, - { - retryFetchErrors: this.config.getRetryFetchErrors(), - }, - ); + const allContent: string[] = []; + const fetchedUrls: string[] = []; + const errors: string[] = []; - const rawContent = await response.text(); - const contentType = response.headers.get('content-type') || ''; - let textContent: string; - - // Only use html-to-text if content type is HTML, or if no content type is provided (assume HTML) - if ( - contentType.toLowerCase().includes('text/html') || - contentType === '' - ) { - textContent = convert(rawContent, { - wordwrap: false, - selectors: [ - { selector: 'a', options: { ignoreHref: true } }, - { selector: 'img', format: 'skip' }, - ], - }); - } else { - // For other content types (text/plain, application/json, etc.), use raw text - textContent = rawContent; + const perUrlBudget = Math.floor(MAX_CONTENT_LENGTH / urls.length); + for (const url of urls) { + try { + const result = await this.executeFallbackForUrl( + url, + signal, + perUrlBudget, + ); + allContent.push(`--- Content from ${url} ---\n${result.content}`); + fetchedUrls.push(url); + } catch (e) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + const error = e as Error; + errors.push(`Error fetching ${url}: ${error.message}`); + } } - textContent = textContent.substring(0, MAX_CONTENT_LENGTH); + if (allContent.length === 0) { + const errorMessage = `Error during fallback fetch: ${errors.join('; ')}`; + return { + llmContent: `Error: ${errorMessage}`, + returnDisplay: `Error: ${errorMessage}`, + error: { + message: errorMessage, + type: ToolErrorType.WEB_FETCH_FALLBACK_FAILED, + }, + }; + } + const combinedContent = allContent.join('\n\n'); const geminiClient = this.config.getGeminiClient(); const fallbackPrompt = `The user requested the following: "${this.params.prompt}". -I was unable to access the URL directly. Instead, I have fetched the raw content of the page. Please use the following content to answer the request. Do not attempt to access the URL again. +I was unable to access the URL(s) directly. Instead, I have fetched the raw content of the page(s). Please use the following content to answer the request. Do not attempt to access the URLs again. ---- -${textContent} ---- +${combinedContent} `; const result = await geminiClient.generateContent( { model: 'web-fetch-fallback' }, @@ -229,14 +262,15 @@ ${textContent} LlmRole.UTILITY_TOOL, ); const resultText = getResponseText(result) || ''; + const displayUrls = fetchedUrls.join(', '); return { llmContent: resultText, - returnDisplay: `Content for ${url} processed using fallback fetch.`, + returnDisplay: `Content for ${displayUrls} processed using fallback fetch.`, }; } catch (e) { // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion const error = e as Error; - const errorMessage = `Error during fallback fetch for ${url}: ${error.message}`; + const errorMessage = `Error during fallback fetch: ${error.message}`; return { llmContent: `Error: ${errorMessage}`, returnDisplay: `Error: ${errorMessage}`, @@ -293,25 +327,28 @@ ${textContent} async execute(signal: AbortSignal): Promise { const userPrompt = this.params.prompt; const { validUrls: urls } = parsePrompt(userPrompt); - const url = urls[0]; - // Enforce rate limiting - const rateLimitResult = checkRateLimit(url); - if (!rateLimitResult.allowed) { - const waitTimeSecs = Math.ceil((rateLimitResult.waitTimeMs || 0) / 1000); - const errorMessage = `Rate limit exceeded for host. Please wait ${waitTimeSecs} seconds before trying again.`; - debugLogger.warn(`[WebFetchTool] Rate limit exceeded for ${url}`); - return { - llmContent: `Error: ${errorMessage}`, - returnDisplay: `Error: ${errorMessage}`, - error: { - message: errorMessage, - type: ToolErrorType.WEB_FETCH_PROCESSING_ERROR, - }, - }; + // Enforce rate limiting for all URLs + for (const url of urls) { + const rateLimitResult = checkRateLimit(url); + if (!rateLimitResult.allowed) { + const waitTimeSecs = Math.ceil( + (rateLimitResult.waitTimeMs || 0) / 1000, + ); + const errorMessage = `Rate limit exceeded for host. Please wait ${waitTimeSecs} seconds before trying again.`; + debugLogger.warn(`[WebFetchTool] Rate limit exceeded for ${url}`); + return { + llmContent: `Error: ${errorMessage}`, + returnDisplay: `Error: ${errorMessage}`, + error: { + message: errorMessage, + type: ToolErrorType.WEB_FETCH_PROCESSING_ERROR, + }, + }; + } } - const isPrivate = isPrivateIp(url); + const isPrivate = urls.some((url) => isPrivateIp(url)); if (isPrivate) { logWebFetchFallbackAttempt( diff --git a/packages/core/src/tools/write-file.test.ts b/packages/core/src/tools/write-file.test.ts index 3a0c8487b8..84fd4d93d7 100644 --- a/packages/core/src/tools/write-file.test.ts +++ b/packages/core/src/tools/write-file.test.ts @@ -310,6 +310,42 @@ describe('WriteFileTool', () => { }; expect(() => tool.build(params)).toThrow(`Missing or empty "file_path"`); }); + + it('should throw an error if content includes an omission placeholder', () => { + const params = { + file_path: path.join(rootDir, 'placeholder.txt'), + content: '(rest of methods ...)', + }; + expect(() => tool.build(params)).toThrow( + "`content` contains an omission placeholder (for example 'rest of methods ...'). Provide complete file content.", + ); + }); + + it('should throw an error when multiline content includes omission placeholders', () => { + const params = { + file_path: path.join(rootDir, 'service.ts'), + content: `class Service { + execute() { + return "run"; + } + + // rest of methods ... +}`, + }; + expect(() => tool.build(params)).toThrow( + "`content` contains an omission placeholder (for example 'rest of methods ...'). Provide complete file content.", + ); + }); + + it('should allow content with placeholder text in a normal string literal', () => { + const params = { + file_path: path.join(rootDir, 'valid-content.ts'), + content: 'const note = "(rest of methods ...)";', + }; + const invocation = tool.build(params); + expect(invocation).toBeDefined(); + expect(invocation.params).toEqual(params); + }); }); describe('getCorrectedFileContent', () => { diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index afdb3587ac..d7708d767a 100644 --- a/packages/core/src/tools/write-file.ts +++ b/packages/core/src/tools/write-file.ts @@ -20,13 +20,9 @@ import type { ToolInvocation, ToolLocation, ToolResult, + ToolConfirmationOutcome, } from './tools.js'; -import { - BaseDeclarativeTool, - BaseToolInvocation, - Kind, - type ToolConfirmationOutcome, -} from './tools.js'; +import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js'; import { ToolErrorType } from './tool-error.js'; import { makeRelative, shortenPath } from '../utils/paths.js'; import { getErrorMessage, isNodeError } from '../utils/errors.js'; @@ -51,6 +47,7 @@ import type { MessageBus } from '../confirmation-bus/message-bus.js'; import { debugLogger } from '../utils/debugLogger.js'; import { WRITE_FILE_DEFINITION } from './definitions/coreTools.js'; import { resolveToolDeclaration } from './definitions/resolver.js'; +import { detectOmissionPlaceholders } from './omissionPlaceholderDetector.js'; /** * Parameters for the WriteFile tool @@ -490,6 +487,11 @@ export class WriteFileTool }`; } + const omissionPlaceholders = detectOmissionPlaceholders(params.content); + if (omissionPlaceholders.length > 0) { + return "`content` contains an omission placeholder (for example 'rest of methods ...'). Provide complete file content."; + } + return null; } diff --git a/packages/core/src/tools/write-todos.ts b/packages/core/src/tools/write-todos.ts index 38aef4f309..5eb42c73f4 100644 --- a/packages/core/src/tools/write-todos.ts +++ b/packages/core/src/tools/write-todos.ts @@ -4,14 +4,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type { ToolInvocation } from './tools.js'; -import { - BaseDeclarativeTool, - BaseToolInvocation, - Kind, - type Todo, - type ToolResult, -} from './tools.js'; +import type { ToolInvocation, Todo, ToolResult } from './tools.js'; +import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import { WRITE_TODOS_TOOL_NAME } from './tool-names.js'; import { WRITE_TODOS_DEFINITION } from './definitions/coreTools.js'; diff --git a/packages/core/src/utils/authConsent.test.ts b/packages/core/src/utils/authConsent.test.ts index c46df2d250..7fc05b2a03 100644 --- a/packages/core/src/utils/authConsent.test.ts +++ b/packages/core/src/utils/authConsent.test.ts @@ -56,6 +56,25 @@ describe('getConsentForOauth', () => { ); }); + it('should handle empty prompt correctly', async () => { + const mockEmitConsentRequest = vi.spyOn(coreEvents, 'emitConsentRequest'); + vi.spyOn(coreEvents, 'listenerCount').mockReturnValue(1); + + mockEmitConsentRequest.mockImplementation((payload) => { + payload.onConfirm(true); + }); + + await getConsentForOauth(''); + + expect(mockEmitConsentRequest).toHaveBeenCalledWith( + expect.objectContaining({ + prompt: expect.stringMatching( + /^Opening authentication page in your browser\./, + ), + }), + ); + }); + it('should return false when user declines via UI', async () => { const mockEmitConsentRequest = vi.spyOn(coreEvents, 'emitConsentRequest'); vi.spyOn(coreEvents, 'listenerCount').mockReturnValue(1); diff --git a/packages/core/src/utils/authConsent.ts b/packages/core/src/utils/authConsent.ts index ef8b52b02e..589c922f57 100644 --- a/packages/core/src/utils/authConsent.ts +++ b/packages/core/src/utils/authConsent.ts @@ -15,7 +15,9 @@ import { isHeadlessMode } from './headless.js'; * Handles both interactive and non-interactive (headless) modes. */ export async function getConsentForOauth(prompt: string): Promise { - const finalPrompt = prompt + ' Opening authentication page in your browser. '; + const finalPrompt = + (prompt ? prompt + ' ' : '') + + 'Opening authentication page in your browser. '; if (isHeadlessMode()) { return getOauthConsentNonInteractive(finalPrompt); diff --git a/packages/core/src/utils/editCorrector.test.ts b/packages/core/src/utils/editCorrector.test.ts index 86e7c61d0f..35b126a5ea 100644 --- a/packages/core/src/utils/editCorrector.test.ts +++ b/packages/core/src/utils/editCorrector.test.ts @@ -5,8 +5,8 @@ */ /* eslint-disable @typescript-eslint/no-explicit-any */ -import type { Mock } from 'vitest'; -import { vi, describe, it, expect, beforeEach, type Mocked } from 'vitest'; +import type { Mock, Mocked } from 'vitest'; +import { vi, describe, it, expect, beforeEach } from 'vitest'; import * as fs from 'node:fs'; import { EDIT_TOOL_NAME } from '../tools/tool-names.js'; import type { BaseLlmClient } from '../core/baseLlmClient.js'; diff --git a/packages/core/src/utils/filesearch/fileSearch.ts b/packages/core/src/utils/filesearch/fileSearch.ts index 21d1e19168..97560f7070 100644 --- a/packages/core/src/utils/filesearch/fileSearch.ts +++ b/packages/core/src/utils/filesearch/fileSearch.ts @@ -149,6 +149,7 @@ class RecursiveFileSearch implements FileSearch { filteredCandidates = await this.fzf .find(pattern) .then((results: Array>) => + // eslint-disable-next-line @typescript-eslint/no-unsafe-return results.map((entry: FzfResultItem) => entry.item), ) .catch(() => { diff --git a/packages/core/src/utils/getFolderStructure.test.ts b/packages/core/src/utils/getFolderStructure.test.ts index e6c0c88cdc..5a9a077e91 100644 --- a/packages/core/src/utils/getFolderStructure.test.ts +++ b/packages/core/src/utils/getFolderStructure.test.ts @@ -6,7 +6,6 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import fsPromises from 'node:fs/promises'; -import * as nodePath from 'node:path'; import * as os from 'node:os'; import { getFolderStructure } from './getFolderStructure.js'; import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; @@ -251,7 +250,7 @@ ${testRootDir}${path.sep} it('should ignore files and folders specified in .gitignore', async () => { await fsPromises.writeFile( - nodePath.join(testRootDir, '.gitignore'), + path.join(testRootDir, '.gitignore'), 'ignored.txt\nnode_modules/\n.gemini/*\n!/.gemini/config.yaml', ); await createTestFile('file1.txt'); @@ -274,7 +273,7 @@ ${testRootDir}${path.sep} it('should not ignore files if respectGitIgnore is false', async () => { await fsPromises.writeFile( - nodePath.join(testRootDir, '.gitignore'), + path.join(testRootDir, '.gitignore'), 'ignored.txt', ); await createTestFile('file1.txt'); @@ -298,7 +297,7 @@ ${testRootDir}${path.sep} describe('with geminiignore', () => { it('should ignore geminiignore files by default', async () => { await fsPromises.writeFile( - nodePath.join(testRootDir, GEMINI_IGNORE_FILE_NAME), + path.join(testRootDir, GEMINI_IGNORE_FILE_NAME), 'ignored.txt\nnode_modules/\n.gemini/\n!/.gemini/config.yaml', ); await createTestFile('file1.txt'); @@ -318,7 +317,7 @@ ${testRootDir}${path.sep} it('should not ignore files if respectGeminiIgnore is false', async () => { await fsPromises.writeFile( - nodePath.join(testRootDir, GEMINI_IGNORE_FILE_NAME), + path.join(testRootDir, GEMINI_IGNORE_FILE_NAME), 'ignored.txt\nnode_modules/\n.gemini/\n!/.gemini/config.yaml', ); await createTestFile('file1.txt'); diff --git a/packages/core/src/utils/memoryDiscovery.test.ts b/packages/core/src/utils/memoryDiscovery.test.ts index 32cf8cabc4..3df110d678 100644 --- a/packages/core/src/utils/memoryDiscovery.test.ts +++ b/packages/core/src/utils/memoryDiscovery.test.ts @@ -22,7 +22,7 @@ import { } from '../tools/memoryTool.js'; import { flattenMemory } from '../config/memory.js'; import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; -import { GEMINI_DIR, normalizePath } from './paths.js'; +import { GEMINI_DIR, normalizePath, homedir as pathsHomedir } from './paths.js'; import type { HierarchicalMemory } from '../config/memory.js'; function flattenResult(result: { @@ -62,8 +62,6 @@ vi.mock('../utils/paths.js', async (importOriginal) => { }; }); -import { homedir as pathsHomedir } from './paths.js'; - describe('memoryDiscovery', () => { const DEFAULT_FOLDER_TRUST = true; let testRootDir: string; diff --git a/packages/core/src/utils/safeJsonStringify.ts b/packages/core/src/utils/safeJsonStringify.ts index 611519d09b..b32a09df27 100644 --- a/packages/core/src/utils/safeJsonStringify.ts +++ b/packages/core/src/utils/safeJsonStringify.ts @@ -27,6 +27,7 @@ export function safeJsonStringify( } seen.add(value); } + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return value; }, space, @@ -60,6 +61,7 @@ export function safeJsonStringifyBooleanValuesOnly(obj: any): string { // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion if ((value as Config) !== null && !configSeen) { configSeen = true; + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return value; } if (typeof value === 'boolean') { diff --git a/packages/core/src/utils/stdio.ts b/packages/core/src/utils/stdio.ts index 22fd2e8447..66abbe6ade 100644 --- a/packages/core/src/utils/stdio.ts +++ b/packages/core/src/utils/stdio.ts @@ -91,8 +91,10 @@ export function createWorkingStdio() { // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const value = Reflect.get(target, prop, receiver); if (typeof value === 'function') { + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return value.bind(target); } + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return value; }, }); @@ -105,8 +107,10 @@ export function createWorkingStdio() { // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const value = Reflect.get(target, prop, receiver); if (typeof value === 'function') { + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return value.bind(target); } + // eslint-disable-next-line @typescript-eslint/no-unsafe-return return value; }, }); diff --git a/schemas/settings.schema.json b/schemas/settings.schema.json index 4c51c13e1b..4fe35583d3 100644 --- a/schemas/settings.schema.json +++ b/schemas/settings.schema.json @@ -131,8 +131,8 @@ "retryFetchErrors": { "title": "Retry Fetch Errors", "description": "Retry on \"exception TypeError: fetch failed sending request\" errors.", - "markdownDescription": "Retry on \"exception TypeError: fetch failed sending request\" errors.\n\n- Category: `General`\n- Requires restart: `no`\n- Default: `false`", - "default": false, + "markdownDescription": "Retry on \"exception TypeError: fetch failed sending request\" errors.\n\n- Category: `General`\n- Requires restart: `no`\n- Default: `true`", + "default": true, "type": "boolean" }, "debugKeystrokeLogging": {