diff options
| -rw-r--r-- | packages/cli/src/ui/hooks/slashCommandProcessor.test.ts | 230 | ||||
| -rw-r--r-- | packages/cli/src/ui/hooks/slashCommandProcessor.ts | 83 | ||||
| -rw-r--r-- | packages/core/src/index.ts | 2 | ||||
| -rw-r--r-- | packages/core/src/tools/edit.ts | 2 | ||||
| -rw-r--r-- | packages/core/src/tools/glob.test.ts | 10 | ||||
| -rw-r--r-- | packages/core/src/tools/mcp-client.ts | 94 | ||||
| -rw-r--r-- | packages/core/src/utils/memoryDiscovery.test.ts | 4 | ||||
| -rw-r--r-- | packages/core/src/utils/memoryDiscovery.ts | 45 | ||||
| -rwxr-xr-x | scripts/sandbox_command.sh | 12 |
9 files changed, 449 insertions, 33 deletions
diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts index 1d33d218..5466e887 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts @@ -56,7 +56,11 @@ import { type SlashCommandActionReturn, } from './slashCommandProcessor.js'; import { MessageType } from '../types.js'; -import { type Config } from '@gemini-code/core'; +import { + type Config, + MCPServerStatus, + getMCPServerStatus, +} from '@gemini-code/core'; import * as ShowMemoryCommandModule from './useShowMemoryCommand.js'; import { GIT_COMMIT_INFO } from '../../generated/git-commit.js'; @@ -378,39 +382,54 @@ Add any other context about the problem here. expect(commandResult).toBe(true); }); - it('should display available tools when tools are found', async () => { - const mockTools = [{ name: 'tool1' }, { name: 'tool2' }]; + it('should display only Gemini CLI tools (filtering out MCP tools)', async () => { + // Create mock tools - some with serverName property (MCP tools) and some without (Gemini CLI tools) + const mockTools = [ + { name: 'tool1' }, + { name: 'tool2' }, + { name: 'mcp_tool1', serverName: 'mcp-server1' }, + { name: 'mcp_tool2', serverName: 'mcp-server1' }, + ]; + mockConfig = { ...mockConfig, getToolRegistry: vi.fn().mockResolvedValue({ getAllTools: vi.fn().mockReturnValue(mockTools), }), } as unknown as Config; + const { handleSlashCommand } = getProcessor(); let commandResult: SlashCommandActionReturn | boolean = false; await act(async () => { commandResult = handleSlashCommand('/tools'); }); + // Should only show tool1 and tool2, not the MCP tools expect(mockAddItem).toHaveBeenNthCalledWith( 2, expect.objectContaining({ type: MessageType.INFO, - text: 'Available tools:\n\ntool1\ntool2', + text: 'Available Gemini CLI tools:\n\ntool1\ntool2', }), expect.any(Number), ); expect(commandResult).toBe(true); }); - it('should display a message when no tools are available', async () => { - const mockTools: unknown[] = []; + it('should display a message when no Gemini CLI tools are available', async () => { + // Only MCP tools available + const mockTools = [ + { name: 'mcp_tool1', serverName: 'mcp-server1' }, + { name: 'mcp_tool2', serverName: 'mcp-server1' }, + ]; + mockConfig = { ...mockConfig, getToolRegistry: vi.fn().mockResolvedValue({ getAllTools: vi.fn().mockReturnValue(mockTools), }), } as unknown as Config; + const { handleSlashCommand } = getProcessor(); let commandResult: SlashCommandActionReturn | boolean = false; await act(async () => { @@ -421,10 +440,207 @@ Add any other context about the problem here. 2, expect.objectContaining({ type: MessageType.INFO, - text: 'Available tools:\n\n', + text: 'Available Gemini CLI tools:\n\n', + }), + expect.any(Number), + ); + expect(commandResult).toBe(true); + }); + }); + + describe('/mcp command', () => { + beforeEach(() => { + // Mock the core module with getMCPServerStatus + vi.mock('@gemini-code/core', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + MCPServerStatus: { + CONNECTED: 'connected', + CONNECTING: 'connecting', + DISCONNECTED: 'disconnected', + }, + getMCPServerStatus: vi.fn(), + }; + }); + }); + + it('should show an error if tool registry is not available', async () => { + mockConfig = { + ...mockConfig, + getToolRegistry: vi.fn().mockResolvedValue(undefined), + } as unknown as Config; + const { handleSlashCommand } = getProcessor(); + let commandResult: SlashCommandActionReturn | boolean = false; + await act(async () => { + commandResult = handleSlashCommand('/mcp'); + }); + + expect(mockAddItem).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + type: MessageType.ERROR, + text: 'Could not retrieve tool registry.', + }), + expect.any(Number), + ); + expect(commandResult).toBe(true); + }); + + it('should display a message when no MCP servers are configured', async () => { + mockConfig = { + ...mockConfig, + getToolRegistry: vi.fn().mockResolvedValue({ + getToolsByServer: vi.fn().mockReturnValue([]), + }), + getMcpServers: vi.fn().mockReturnValue({}), + } as unknown as Config; + + const { handleSlashCommand } = getProcessor(); + let commandResult: SlashCommandActionReturn | boolean = false; + await act(async () => { + commandResult = handleSlashCommand('/mcp'); + }); + + expect(mockAddItem).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + type: MessageType.INFO, + text: 'No MCP servers configured.', + }), + expect.any(Number), + ); + expect(commandResult).toBe(true); + }); + + it('should display configured MCP servers with status indicators and their tools', async () => { + // Mock MCP servers configuration + const mockMcpServers = { + server1: { command: 'cmd1' }, + server2: { command: 'cmd2' }, + server3: { command: 'cmd3' }, + }; + + // Setup getMCPServerStatus mock implementation + vi.mocked(getMCPServerStatus).mockImplementation((serverName) => { + if (serverName === 'server1') return MCPServerStatus.CONNECTED; + if (serverName === 'server2') return MCPServerStatus.CONNECTING; + return MCPServerStatus.DISCONNECTED; // Default for server3 and others + }); + + // Mock tools from each server + const mockServer1Tools = [ + { name: 'server1_tool1' }, + { name: 'server1_tool2' }, + ]; + + const mockServer2Tools = [{ name: 'server2_tool1' }]; + + const mockServer3Tools = [{ name: 'server3_tool1' }]; + + const mockGetToolsByServer = vi.fn().mockImplementation((serverName) => { + if (serverName === 'server1') return mockServer1Tools; + if (serverName === 'server2') return mockServer2Tools; + if (serverName === 'server3') return mockServer3Tools; + return []; + }); + + mockConfig = { + ...mockConfig, + getToolRegistry: vi.fn().mockResolvedValue({ + getToolsByServer: mockGetToolsByServer, + }), + getMcpServers: vi.fn().mockReturnValue(mockMcpServers), + } as unknown as Config; + + const { handleSlashCommand } = getProcessor(); + let commandResult: SlashCommandActionReturn | boolean = false; + await act(async () => { + commandResult = handleSlashCommand('/mcp'); + }); + + expect(mockAddItem).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + type: MessageType.INFO, + text: expect.stringContaining('Configured MCP servers and tools:'), }), expect.any(Number), ); + + // Check that the message contains details about both servers and their tools + const message = mockAddItem.mock.calls[1][0].text; + // Server 1 - Connected (green dot) + expect(message).toContain('🟢 server1 (2 tools):'); + expect(message).toContain('server1_tool1'); + expect(message).toContain('server1_tool2'); + + // Server 2 - Connecting (yellow dot) + expect(message).toContain('🟡 server2 (1 tools):'); + expect(message).toContain('server2_tool1'); + + // Server 3 - No status, should default to Disconnected (red dot) + expect(message).toContain('🔴 server3 (1 tools):'); + expect(message).toContain('server3_tool1'); + + expect(commandResult).toBe(true); + }); + + it('should indicate when a server has no tools', async () => { + // Mock MCP servers configuration + const mockMcpServers = { + server1: { command: 'cmd1' }, + server2: { command: 'cmd2' }, + }; + + // Setup getMCPServerStatus mock implementation + vi.mocked(getMCPServerStatus).mockImplementation((serverName) => { + if (serverName === 'server1') return MCPServerStatus.CONNECTED; + if (serverName === 'server2') return MCPServerStatus.DISCONNECTED; + return MCPServerStatus.DISCONNECTED; + }); + + // Mock tools from each server - server2 has no tools + const mockServer1Tools = [{ name: 'server1_tool1' }]; + + const mockServer2Tools = []; + + const mockGetToolsByServer = vi.fn().mockImplementation((serverName) => { + if (serverName === 'server1') return mockServer1Tools; + if (serverName === 'server2') return mockServer2Tools; + return []; + }); + + mockConfig = { + ...mockConfig, + getToolRegistry: vi.fn().mockResolvedValue({ + getToolsByServer: mockGetToolsByServer, + }), + getMcpServers: vi.fn().mockReturnValue(mockMcpServers), + } as unknown as Config; + + const { handleSlashCommand } = getProcessor(); + let commandResult: SlashCommandActionReturn | boolean = false; + await act(async () => { + commandResult = handleSlashCommand('/mcp'); + }); + + expect(mockAddItem).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + type: MessageType.INFO, + text: expect.stringContaining('Configured MCP servers and tools:'), + }), + expect.any(Number), + ); + + // Check that the message contains details about both servers and their tools + const message = mockAddItem.mock.calls[1][0].text; + expect(message).toContain('🟢 server1 (1 tools):'); + expect(message).toContain('server1_tool1'); + expect(message).toContain('🔴 server2 (0 tools):'); + expect(message).toContain('No tools available'); + expect(commandResult).toBe(true); }); }); diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.ts index 01e04d70..68f53873 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.ts @@ -9,7 +9,7 @@ import { type PartListUnion } from '@google/genai'; import open from 'open'; import process from 'node:process'; import { UseHistoryManagerReturn } from './useHistoryManager.js'; -import { Config } from '@gemini-code/core'; +import { Config, MCPServerStatus, getMCPServerStatus } from '@gemini-code/core'; import { Message, MessageType, HistoryItemWithoutId } from '../types.js'; import { createShowMemoryAction } from './useShowMemoryCommand.js'; import { GIT_COMMIT_INFO } from '../../generated/git-commit.js'; @@ -138,9 +138,74 @@ export const useSlashCommandProcessor = ( }, }, { + name: 'mcp', + description: 'list configured MCP servers and tools', + action: async (_mainCommand, _subCommand, _args) => { + const toolRegistry = await config?.getToolRegistry(); + if (!toolRegistry) { + addMessage({ + type: MessageType.ERROR, + content: 'Could not retrieve tool registry.', + timestamp: new Date(), + }); + return; + } + + const mcpServers = config?.getMcpServers() || {}; + const serverNames = Object.keys(mcpServers); + + if (serverNames.length === 0) { + addMessage({ + type: MessageType.INFO, + content: 'No MCP servers configured.', + timestamp: new Date(), + }); + return; + } + + let message = 'Configured MCP servers and tools:\n\n'; + + for (const serverName of serverNames) { + const serverTools = toolRegistry.getToolsByServer(serverName); + const status = getMCPServerStatus(serverName); + + // Add status indicator + let statusDot = ''; + switch (status) { + case MCPServerStatus.CONNECTED: + statusDot = '🟢'; // Green dot for connected + break; + case MCPServerStatus.CONNECTING: + statusDot = '🟡'; // Yellow dot for connecting + break; + case MCPServerStatus.DISCONNECTED: + default: + statusDot = '🔴'; // Red dot for disconnected + break; + } + + message += `${statusDot} ${serverName} (${serverTools.length} tools):\n`; + if (serverTools.length > 0) { + serverTools.forEach((tool) => { + message += ` - ${tool.name}\n`; + }); + } else { + message += ' No tools available\n'; + } + message += '\n'; + } + + addMessage({ + type: MessageType.INFO, + content: message, + timestamp: new Date(), + }); + }, + }, + { name: 'memory', description: - 'Manage memory. Usage: /memory <show|refresh|add> [text for add]', + 'manage memory. Usage: /memory <show|refresh|add> [text for add]', action: (mainCommand, subCommand, args) => { switch (subCommand) { case 'show': @@ -163,7 +228,7 @@ export const useSlashCommandProcessor = ( }, { name: 'tools', - description: 'list available tools', + description: 'list available Gemini CLI tools', action: async (_mainCommand, _subCommand, _args) => { const toolRegistry = await config?.getToolRegistry(); const tools = toolRegistry?.getAllTools(); @@ -175,10 +240,14 @@ export const useSlashCommandProcessor = ( }); return; } - const toolList = tools.map((tool) => tool.name); + + // Filter out MCP tools by checking if they have a serverName property + const geminiTools = tools.filter((tool) => !('serverName' in tool)); + const geminiToolList = geminiTools.map((tool) => tool.name); + addMessage({ type: MessageType.INFO, - content: `Available tools:\n\n${toolList.join('\n')}`, + content: `Available Gemini CLI tools:\n\n${geminiToolList.join('\n')}`, timestamp: new Date(), }); }, @@ -191,7 +260,7 @@ export const useSlashCommandProcessor = ( }, { name: 'about', - description: 'Show version info', + description: 'show version info', action: (_mainCommand, _subCommand, _args) => { const osVersion = process.platform; let sandboxEnv = 'no sandbox'; @@ -214,7 +283,7 @@ export const useSlashCommandProcessor = ( }, { name: 'bug', - description: 'Submit a bug report.', + description: 'submit a bug report', action: (_mainCommand, _subCommand, args) => { let bugDescription = _subCommand || ''; if (args) { diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index a285df14..1f58d97c 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -44,6 +44,8 @@ export * from './tools/memoryTool.js'; export * from './tools/shell.js'; export * from './tools/web-search.js'; export * from './tools/read-many-files.js'; +export * from './tools/mcp-client.js'; +export * from './tools/mcp-tool.js'; // Export telemetry functions export * from './telemetry/index.js'; diff --git a/packages/core/src/tools/edit.ts b/packages/core/src/tools/edit.ts index bdaa805c..eb22d0ac 100644 --- a/packages/core/src/tools/edit.ts +++ b/packages/core/src/tools/edit.ts @@ -91,7 +91,7 @@ Expectation for required parameters: 2. \`old_string\` MUST be the exact literal text to replace (including all whitespace, indentation, newlines, and surrounding code etc.). 3. \`new_string\` MUST be the exact literal text to replace \`old_string\` with (also including all whitespace, indentation, newlines, and surrounding code etc.). Ensure the resulting code is correct and idiomatic. 4. NEVER escape \`old_string\` or \`new_string\`, that would break the exact literal text requirement. -**Important:** If ANY of the above are not satisfied, the tool will fail. CRITICAL for \`old_string\`: Must uniquely identify the single instance to change. Include at least 3 lines of context BEFORE and AFTER the target text, matching whitespace and indentation precisely. If this string matches multiple locations, or does not match exactly, the tool will fail., +**Important:** If ANY of the above are not satisfied, the tool will fail. CRITICAL for \`old_string\`: Must uniquely identify the single instance to change. Include at least 3 lines of context BEFORE and AFTER the target text, matching whitespace and indentation precisely. If this string matches multiple locations, or does not match exactly, the tool will fail. **Multiple replacements:** Set \`expected_replacements\` to the number of occurrences you want to replace. The tool will replace ALL occurrences that match \`old_string\` exactly. Ensure the number of replacements matches your expectation.`, { properties: { diff --git a/packages/core/src/tools/glob.test.ts b/packages/core/src/tools/glob.test.ts index b630a0d8..f975fc08 100644 --- a/packages/core/src/tools/glob.test.ts +++ b/packages/core/src/tools/glob.test.ts @@ -185,7 +185,9 @@ describe('GlobTool', () => { }); it('should return error if pattern is missing (schema validation)', () => { - const params = { path: '.' } as unknown as GlobToolParams; + // Need to correctly define this as an object without pattern + const params = { path: '.' }; + // @ts-expect-error - We're intentionally creating invalid params for testing expect(globTool.validateToolParams(params)).toContain( 'Parameters failed schema validation', ); @@ -209,7 +211,8 @@ describe('GlobTool', () => { const params = { pattern: '*.ts', path: 123, - } as unknown as GlobToolParams; + }; + // @ts-expect-error - We're intentionally creating invalid params for testing expect(globTool.validateToolParams(params)).toContain( 'Parameters failed schema validation', ); @@ -219,7 +222,8 @@ describe('GlobTool', () => { const params = { pattern: '*.ts', case_sensitive: 'true', - } as unknown as GlobToolParams; + }; + // @ts-expect-error - We're intentionally creating invalid params for testing expect(globTool.validateToolParams(params)).toContain( 'Parameters failed schema validation', ); diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts index 9025ac7b..9a02df0c 100644 --- a/packages/core/src/tools/mcp-client.ts +++ b/packages/core/src/tools/mcp-client.ts @@ -13,6 +13,83 @@ import { DiscoveredMCPTool } from './mcp-tool.js'; import { CallableTool, FunctionDeclaration, mcpToTool } from '@google/genai'; import { ToolRegistry } from './tool-registry.js'; +/** + * Enum representing the connection status of an MCP server + */ +export enum MCPServerStatus { + /** Server is disconnected or experiencing errors */ + DISCONNECTED = 'disconnected', + /** Server is in the process of connecting */ + CONNECTING = 'connecting', + /** Server is connected and ready to use */ + CONNECTED = 'connected', +} + +/** + * Map to track the status of each MCP server within the core package + */ +const mcpServerStatusesInternal: Map<string, MCPServerStatus> = new Map(); + +/** + * Event listeners for MCP server status changes + */ +type StatusChangeListener = ( + serverName: string, + status: MCPServerStatus, +) => void; +const statusChangeListeners: StatusChangeListener[] = []; + +/** + * Add a listener for MCP server status changes + */ +export function addMCPStatusChangeListener( + listener: StatusChangeListener, +): void { + statusChangeListeners.push(listener); +} + +/** + * Remove a listener for MCP server status changes + */ +export function removeMCPStatusChangeListener( + listener: StatusChangeListener, +): void { + const index = statusChangeListeners.indexOf(listener); + if (index !== -1) { + statusChangeListeners.splice(index, 1); + } +} + +/** + * Update the status of an MCP server + */ +function updateMCPServerStatus( + serverName: string, + status: MCPServerStatus, +): void { + mcpServerStatusesInternal.set(serverName, status); + // Notify all listeners + for (const listener of statusChangeListeners) { + listener(serverName, status); + } +} + +/** + * Get the current status of an MCP server + */ +export function getMCPServerStatus(serverName: string): MCPServerStatus { + return ( + mcpServerStatusesInternal.get(serverName) || MCPServerStatus.DISCONNECTED + ); +} + +/** + * Get all MCP server statuses + */ +export function getAllMCPServerStatuses(): Map<string, MCPServerStatus> { + return new Map(mcpServerStatusesInternal); +} + export async function discoverMcpTools( mcpServers: Record<string, MCPServerConfig>, mcpServerCommand: string | undefined, @@ -43,6 +120,9 @@ async function connectAndDiscover( mcpServerConfig: MCPServerConfig, toolRegistry: ToolRegistry, ): Promise<void> { + // Initialize the server status as connecting + updateMCPServerStatus(mcpServerName, MCPServerStatus.CONNECTING); + let transport; if (mcpServerConfig.url) { transport = new SSEClientTransport(new URL(mcpServerConfig.url)); @@ -61,6 +141,8 @@ async function connectAndDiscover( console.error( `MCP server '${mcpServerName}' has invalid configuration: missing both url (for SSE) and command (for stdio). Skipping.`, ); + // Update status to disconnected + updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED); return; } @@ -72,16 +154,22 @@ async function connectAndDiscover( try { await mcpClient.connect(transport); + // Connection successful + updateMCPServerStatus(mcpServerName, MCPServerStatus.CONNECTED); } catch (error) { console.error( `failed to start or connect to MCP server '${mcpServerName}' ` + `${JSON.stringify(mcpServerConfig)}; \n${error}`, ); + // Update status to disconnected + updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED); return; } mcpClient.onerror = (error) => { console.error(`MCP ERROR (${mcpServerName}):`, error.toString()); + // Update status to disconnected on error + updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED); }; if (transport instanceof StdioClientTransport && transport.stderr) { @@ -110,6 +198,8 @@ async function connectAndDiscover( } else if (transport instanceof SSEClientTransport) { await transport.close(); } + // Update status to disconnected + updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED); return; } @@ -168,6 +258,8 @@ async function connectAndDiscover( ) { await transport.close(); } + // Update status to disconnected + updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED); } // If no tools were registered from this MCP server, the following 'if' block @@ -184,6 +276,8 @@ async function connectAndDiscover( transport instanceof SSEClientTransport ) { await transport.close(); + // Update status to disconnected + updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED); } } } diff --git a/packages/core/src/utils/memoryDiscovery.test.ts b/packages/core/src/utils/memoryDiscovery.test.ts index db0ffd1d..a9d34bf3 100644 --- a/packages/core/src/utils/memoryDiscovery.test.ts +++ b/packages/core/src/utils/memoryDiscovery.test.ts @@ -45,6 +45,10 @@ describe('loadServerHierarchicalMemory', () => { beforeEach(() => { vi.resetAllMocks(); + // Set environment variables to indicate test environment + process.env.NODE_ENV = 'test'; + process.env.VITEST = 'true'; + setGeminiMdFilename(DEFAULT_CONTEXT_FILENAME); // Use defined const mockOs.homedir.mockReturnValue(USER_HOME); diff --git a/packages/core/src/utils/memoryDiscovery.ts b/packages/core/src/utils/memoryDiscovery.ts index 2e6ce9fc..221bf2c6 100644 --- a/packages/core/src/utils/memoryDiscovery.ts +++ b/packages/core/src/utils/memoryDiscovery.ts @@ -56,17 +56,29 @@ async function findProjectRoot(startDir: string): Promise<string | null> { return currentDir; } } catch (error: unknown) { - if (typeof error === 'object' && error !== null && 'code' in error) { - const fsError = error as { code: string; message: string }; - if (fsError.code !== 'ENOENT') { + // Don't log ENOENT errors as they're expected when .git doesn't exist + // Also don't log errors in test environments, which often have mocked fs + const isENOENT = + typeof error === 'object' && + error !== null && + 'code' in error && + (error as { code: string }).code === 'ENOENT'; + + // Only log unexpected errors in non-test environments + // process.env.NODE_ENV === 'test' or VITEST are common test indicators + const isTestEnv = process.env.NODE_ENV === 'test' || process.env.VITEST; + + if (!isENOENT && !isTestEnv) { + if (typeof error === 'object' && error !== null && 'code' in error) { + const fsError = error as { code: string; message: string }; logger.warn( `Error checking for .git directory at ${gitPath}: ${fsError.message}`, ); + } else { + logger.warn( + `Non-standard error checking for .git directory at ${gitPath}: ${String(error)}`, + ); } - } else { - logger.warn( - `Non-standard error checking for .git directory at ${gitPath}: ${String(error)}`, - ); } } const parentDir = path.dirname(currentDir); @@ -136,8 +148,12 @@ async function collectDownwardGeminiFiles( } } } catch (error) { - const message = error instanceof Error ? error.message : String(error); - logger.warn(`Error scanning directory ${directory}: ${message}`); + // Only log warnings in non-test environments + const isTestEnv = process.env.NODE_ENV === 'test' || process.env.VITEST; + if (!isTestEnv) { + const message = error instanceof Error ? error.message : String(error); + logger.warn(`Error scanning directory ${directory}: ${message}`); + } if (debugMode) logger.debug(`Failed to scan directory: ${directory}`); } return collectedPaths; @@ -283,10 +299,13 @@ async function readGeminiMdFiles( `Successfully read: ${filePath} (Length: ${content.length})`, ); } catch (error: unknown) { - const message = error instanceof Error ? error.message : String(error); - logger.warn( - `Warning: Could not read ${getCurrentGeminiMdFilename()} file at ${filePath}. Error: ${message}`, - ); + const isTestEnv = process.env.NODE_ENV === 'test' || process.env.VITEST; + if (!isTestEnv) { + const message = error instanceof Error ? error.message : String(error); + logger.warn( + `Warning: Could not read ${getCurrentGeminiMdFilename()} file at ${filePath}. Error: ${message}`, + ); + } results.push({ filePath, content: null }); // Still include it with null content if (debugMode) logger.debug(`Failed to read: ${filePath}`); } diff --git a/scripts/sandbox_command.sh b/scripts/sandbox_command.sh index 325722f5..468a4834 100755 --- a/scripts/sandbox_command.sh +++ b/scripts/sandbox_command.sh @@ -35,8 +35,16 @@ shift $((OPTIND - 1)) # note it can be string or boolean, and if missing `npx json` will return empty string USER_SETTINGS_FILE="$HOME/.gemini/settings.json" if [ -z "${GEMINI_SANDBOX:-}" ] && [ -f "$USER_SETTINGS_FILE" ]; then - USER_SANDBOX_SETTING=$(sed -e 's/\/\/.*//' -e 's/\/\*.*\*\///g' -e '/^[[:space:]]*\/\//d' "$USER_SETTINGS_FILE" | npx json 'sandbox') - if [ -n "$USER_SANDBOX_SETTING" ]; then + # Check if jq is available (more reliable than npx json) + if command -v jq &>/dev/null; then + USER_SANDBOX_SETTING=$(jq -r '.sandbox // empty' "$USER_SETTINGS_FILE" 2>/dev/null || echo "") + else + # Fallback to npx json with error handling + USER_SANDBOX_SETTING=$(sed -e 's/\/\/.*//' -e 's/\/\*.*\*\///g' -e '/^[[:space:]]*\/\//d' "$USER_SETTINGS_FILE" | npx json 'sandbox' 2>/dev/null || echo "") + fi + + # Avoid setting GEMINI_SANDBOX to complex objects + if [ -n "$USER_SANDBOX_SETTING" ] && [[ ! "$USER_SANDBOX_SETTING" =~ ^\{.*\}$ ]]; then GEMINI_SANDBOX=$USER_SANDBOX_SETTING fi fi |