From 67d16992cfc6b18f1d242865e0fbd8f5f40cf25f Mon Sep 17 00:00:00 2001 From: joshualitt Date: Fri, 1 Aug 2025 12:30:39 -0700 Subject: [PATCH] bug(cli): Prefer IPv4 dns resolution by default. (#5338) --- packages/cli/src/config/settings.test.ts | 42 ++++++++++++++++++++++++ packages/cli/src/config/settings.ts | 3 ++ packages/cli/src/gemini.test.tsx | 41 ++++++++++++++++++++++- packages/cli/src/gemini.tsx | 23 +++++++++++++ 4 files changed, 108 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index ae655fe1..5a54e46e 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -777,6 +777,48 @@ describe('Settings Loading and Merging', () => { } }); + it('should correctly merge dnsResolutionOrder with workspace taking precedence', () => { + (mockFsExistsSync as Mock).mockReturnValue(true); + const userSettingsContent = { + dnsResolutionOrder: 'ipv4first', + }; + const workspaceSettingsContent = { + dnsResolutionOrder: 'verbatim', + }; + + (fs.readFileSync as Mock).mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p === USER_SETTINGS_PATH) + return JSON.stringify(userSettingsContent); + if (p === MOCK_WORKSPACE_SETTINGS_PATH) + return JSON.stringify(workspaceSettingsContent); + return '{}'; + }, + ); + + const settings = loadSettings(MOCK_WORKSPACE_DIR); + expect(settings.merged.dnsResolutionOrder).toBe('verbatim'); + }); + + it('should use user dnsResolutionOrder if workspace is not defined', () => { + (mockFsExistsSync as Mock).mockImplementation( + (p: fs.PathLike) => p === USER_SETTINGS_PATH, + ); + const userSettingsContent = { + dnsResolutionOrder: 'verbatim', + }; + (fs.readFileSync as Mock).mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p === USER_SETTINGS_PATH) + return JSON.stringify(userSettingsContent); + return '{}'; + }, + ); + + const settings = loadSettings(MOCK_WORKSPACE_DIR); + expect(settings.merged.dnsResolutionOrder).toBe('verbatim'); + }); + it('should leave unresolved environment variables as is', () => { const userSettingsContent = { apiKey: '$UNDEFINED_VAR' }; (mockFsExistsSync as Mock).mockImplementation( diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index 76eb1745..4f701c6d 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -38,6 +38,8 @@ export function getSystemSettingsPath(): string { } } +export type DnsResolutionOrder = 'ipv4first' | 'verbatim'; + export enum SettingScope { User = 'User', Workspace = 'Workspace', @@ -110,6 +112,7 @@ export interface Settings { disableAutoUpdate?: boolean; memoryDiscoveryMaxDirs?: number; + dnsResolutionOrder?: DnsResolutionOrder; } export interface SettingsError { diff --git a/packages/cli/src/gemini.test.tsx b/packages/cli/src/gemini.test.tsx index 505841c7..c8bb45ab 100644 --- a/packages/cli/src/gemini.test.tsx +++ b/packages/cli/src/gemini.test.tsx @@ -6,7 +6,11 @@ import stripAnsi from 'strip-ansi'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { main, setupUnhandledRejectionHandler } from './gemini.js'; +import { + main, + setupUnhandledRejectionHandler, + validateDnsResolutionOrder, +} from './gemini.js'; import { LoadedSettings, SettingsFile, @@ -211,3 +215,38 @@ describe('gemini.tsx main function', () => { processExitSpy.mockRestore(); }); }); + +describe('validateDnsResolutionOrder', () => { + let consoleWarnSpy: ReturnType; + + beforeEach(() => { + consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + + afterEach(() => { + consoleWarnSpy.mockRestore(); + }); + + it('should return "ipv4first" when the input is "ipv4first"', () => { + expect(validateDnsResolutionOrder('ipv4first')).toBe('ipv4first'); + expect(consoleWarnSpy).not.toHaveBeenCalled(); + }); + + it('should return "verbatim" when the input is "verbatim"', () => { + expect(validateDnsResolutionOrder('verbatim')).toBe('verbatim'); + expect(consoleWarnSpy).not.toHaveBeenCalled(); + }); + + it('should return the default "ipv4first" when the input is undefined', () => { + expect(validateDnsResolutionOrder(undefined)).toBe('ipv4first'); + expect(consoleWarnSpy).not.toHaveBeenCalled(); + }); + + it('should return the default "ipv4first" and log a warning for an invalid string', () => { + expect(validateDnsResolutionOrder('invalid-value')).toBe('ipv4first'); + expect(consoleWarnSpy).toHaveBeenCalledOnce(); + expect(consoleWarnSpy).toHaveBeenCalledWith( + 'Invalid value for dnsResolutionOrder in settings: "invalid-value". Using default "ipv4first".', + ); + }); +}); diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 73f3fdd0..48dbd271 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -12,9 +12,11 @@ import { readStdin } from './utils/readStdin.js'; import { basename } from 'node:path'; import v8 from 'node:v8'; import os from 'node:os'; +import dns from 'node:dns'; import { spawn } from 'node:child_process'; import { start_sandbox } from './utils/sandbox.js'; import { + DnsResolutionOrder, LoadedSettings, loadSettings, SettingScope, @@ -44,6 +46,23 @@ import { checkForUpdates } from './ui/utils/updateCheck.js'; import { handleAutoUpdate } from './utils/handleAutoUpdate.js'; import { appEvents, AppEvent } from './utils/events.js'; +export function validateDnsResolutionOrder( + order: string | undefined, +): DnsResolutionOrder { + const defaultValue: DnsResolutionOrder = 'ipv4first'; + if (order === undefined) { + return defaultValue; + } + if (order === 'ipv4first' || order === 'verbatim') { + return order; + } + // We don't want to throw here, just warn and use the default. + console.warn( + `Invalid value for dnsResolutionOrder in settings: "${order}". Using default "${defaultValue}".`, + ); + return defaultValue; +} + function getNodeMemoryArgs(config: Config): string[] { const totalMemoryMB = os.totalmem() / (1024 * 1024); const heapStats = v8.getHeapStatistics(); @@ -138,6 +157,10 @@ export async function main() { argv, ); + dns.setDefaultResultOrder( + validateDnsResolutionOrder(settings.merged.dnsResolutionOrder), + ); + if (argv.promptInteractive && !process.stdin.isTTY) { console.error( 'Error: The --prompt-interactive flag is not supported when piping input from stdin.',