fix(config): Resolve duplicate config loading from home directory (#5090)

Co-authored-by: Allen Hutchison <adh@google.com>
Co-authored-by: Allen Hutchison <allen@hutchison.org>
This commit is contained in:
TIRUMALASETTI PRANITH 2025-08-02 03:52:17 +05:30 committed by GitHub
parent 387706607d
commit 15a1f1af9d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 117 additions and 110 deletions

View File

@ -4,6 +4,9 @@
* SPDX-License-Identifier: Apache-2.0 * SPDX-License-Identifier: Apache-2.0
*/ */
import * as fs from 'fs';
import * as path from 'path';
import { homedir } from 'node:os';
import yargs from 'yargs/yargs'; import yargs from 'yargs/yargs';
import { hideBin } from 'yargs/helpers'; import { hideBin } from 'yargs/helpers';
import process from 'node:process'; import process from 'node:process';
@ -244,16 +247,24 @@ export async function loadHierarchicalGeminiMemory(
memoryImportFormat: 'flat' | 'tree' = 'tree', memoryImportFormat: 'flat' | 'tree' = 'tree',
fileFilteringOptions?: FileFilteringOptions, fileFilteringOptions?: FileFilteringOptions,
): Promise<{ memoryContent: string; fileCount: number }> { ): Promise<{ memoryContent: string; fileCount: number }> {
// FIX: Use real, canonical paths for a reliable comparison to handle symlinks.
const realCwd = fs.realpathSync(path.resolve(currentWorkingDirectory));
const realHome = fs.realpathSync(path.resolve(homedir()));
const isHomeDirectory = realCwd === realHome;
// If it is the home directory, pass an empty string to the core memory
// function to signal that it should skip the workspace search.
const effectiveCwd = isHomeDirectory ? '' : currentWorkingDirectory;
if (debugMode) { if (debugMode) {
logger.debug( logger.debug(
`CLI: Delegating hierarchical memory load to server for CWD: ${currentWorkingDirectory} (memoryImportFormat: ${memoryImportFormat})`, `CLI: Delegating hierarchical memory load to server for CWD: ${currentWorkingDirectory} (memoryImportFormat: ${memoryImportFormat})`,
); );
} }
// Directly call the server function. // Directly call the server function with the corrected path.
// The server function will use its own homedir() for the global path.
return loadServerHierarchicalMemory( return loadServerHierarchicalMemory(
currentWorkingDirectory, effectiveCwd,
debugMode, debugMode,
fileService, fileService,
extensionContextFilePaths, extensionContextFilePaths,

View File

@ -59,7 +59,21 @@ const MOCK_WORKSPACE_SETTINGS_PATH = pathActual.join(
'settings.json', 'settings.json',
); );
vi.mock('fs'); vi.mock('fs', async (importOriginal) => {
// Get all the functions from the real 'fs' module
const actualFs = await importOriginal<typeof fs>();
return {
...actualFs, // Keep all the real functions
// Now, just override the ones we need for the test
existsSync: vi.fn(),
readFileSync: vi.fn(),
writeFileSync: vi.fn(),
mkdirSync: vi.fn(),
realpathSync: (p: string) => p,
};
});
vi.mock('strip-json-comments', () => ({ vi.mock('strip-json-comments', () => ({
default: vi.fn((content) => content), default: vi.fn((content) => content),
})); }));

View File

@ -312,6 +312,22 @@ export function loadSettings(workspaceDir: string): LoadedSettings {
let workspaceSettings: Settings = {}; let workspaceSettings: Settings = {};
const settingsErrors: SettingsError[] = []; const settingsErrors: SettingsError[] = [];
const systemSettingsPath = getSystemSettingsPath(); const systemSettingsPath = getSystemSettingsPath();
// FIX: Resolve paths to their canonical representation to handle symlinks
const resolvedWorkspaceDir = path.resolve(workspaceDir);
const resolvedHomeDir = path.resolve(homedir());
let realWorkspaceDir = resolvedWorkspaceDir;
try {
// fs.realpathSync gets the "true" path, resolving any symlinks
realWorkspaceDir = fs.realpathSync(resolvedWorkspaceDir);
} catch (_e) {
// This is okay. The path might not exist yet, and that's a valid state.
}
// We expect homedir to always exist and be resolvable.
const realHomeDir = fs.realpathSync(resolvedHomeDir);
// Load system settings // Load system settings
try { try {
if (fs.existsSync(systemSettingsPath)) { if (fs.existsSync(systemSettingsPath)) {
@ -356,28 +372,31 @@ export function loadSettings(workspaceDir: string): LoadedSettings {
'settings.json', 'settings.json',
); );
// Load workspace settings // This comparison is now much more reliable.
try { if (realWorkspaceDir !== realHomeDir) {
if (fs.existsSync(workspaceSettingsPath)) { // Load workspace settings
const projectContent = fs.readFileSync(workspaceSettingsPath, 'utf-8'); try {
const parsedWorkspaceSettings = JSON.parse( if (fs.existsSync(workspaceSettingsPath)) {
stripJsonComments(projectContent), const projectContent = fs.readFileSync(workspaceSettingsPath, 'utf-8');
) as Settings; const parsedWorkspaceSettings = JSON.parse(
workspaceSettings = resolveEnvVarsInObject(parsedWorkspaceSettings); stripJsonComments(projectContent),
if (workspaceSettings.theme && workspaceSettings.theme === 'VS') { ) as Settings;
workspaceSettings.theme = DefaultLight.name; workspaceSettings = resolveEnvVarsInObject(parsedWorkspaceSettings);
} else if ( if (workspaceSettings.theme && workspaceSettings.theme === 'VS') {
workspaceSettings.theme && workspaceSettings.theme = DefaultLight.name;
workspaceSettings.theme === 'VS2015' } else if (
) { workspaceSettings.theme &&
workspaceSettings.theme = DefaultDark.name; workspaceSettings.theme === 'VS2015'
) {
workspaceSettings.theme = DefaultDark.name;
}
} }
} catch (error: unknown) {
settingsErrors.push({
message: getErrorMessage(error),
path: workspaceSettingsPath,
});
} }
} catch (error: unknown) {
settingsErrors.push({
message: getErrorMessage(error),
path: workspaceSettingsPath,
});
} }
return new LoadedSettings( return new LoadedSettings(

View File

@ -94,7 +94,6 @@ async function getGeminiMdFilePathsInternal(
const geminiMdFilenames = getAllGeminiMdFilenames(); const geminiMdFilenames = getAllGeminiMdFilenames();
for (const geminiMdFilename of geminiMdFilenames) { for (const geminiMdFilename of geminiMdFilenames) {
const resolvedCwd = path.resolve(currentWorkingDirectory);
const resolvedHome = path.resolve(userHomePath); const resolvedHome = path.resolve(userHomePath);
const globalMemoryPath = path.join( const globalMemoryPath = path.join(
resolvedHome, resolvedHome,
@ -102,12 +101,7 @@ async function getGeminiMdFilePathsInternal(
geminiMdFilename, geminiMdFilename,
); );
if (debugMode) // This part that finds the global file always runs.
logger.debug(
`Searching for ${geminiMdFilename} starting from CWD: ${resolvedCwd}`,
);
if (debugMode) logger.debug(`User home directory: ${resolvedHome}`);
try { try {
await fs.access(globalMemoryPath, fsSync.constants.R_OK); await fs.access(globalMemoryPath, fsSync.constants.R_OK);
allPaths.add(globalMemoryPath); allPaths.add(globalMemoryPath);
@ -116,102 +110,71 @@ async function getGeminiMdFilePathsInternal(
`Found readable global ${geminiMdFilename}: ${globalMemoryPath}`, `Found readable global ${geminiMdFilename}: ${globalMemoryPath}`,
); );
} catch { } catch {
// It's okay if it's not found.
}
// FIX: Only perform the workspace search (upward and downward scans)
// if a valid currentWorkingDirectory is provided.
if (currentWorkingDirectory) {
const resolvedCwd = path.resolve(currentWorkingDirectory);
if (debugMode) if (debugMode)
logger.debug( logger.debug(
`Global ${geminiMdFilename} not found or not readable: ${globalMemoryPath}`, `Searching for ${geminiMdFilename} starting from CWD: ${resolvedCwd}`,
); );
}
const projectRoot = await findProjectRoot(resolvedCwd); const projectRoot = await findProjectRoot(resolvedCwd);
if (debugMode) if (debugMode)
logger.debug(`Determined project root: ${projectRoot ?? 'None'}`); logger.debug(`Determined project root: ${projectRoot ?? 'None'}`);
const upwardPaths: string[] = []; const upwardPaths: string[] = [];
let currentDir = resolvedCwd; let currentDir = resolvedCwd;
// Determine the directory that signifies the top of the project or user-specific space. const ultimateStopDir = projectRoot
const ultimateStopDir = projectRoot ? path.dirname(projectRoot)
? path.dirname(projectRoot) : path.dirname(resolvedHome);
: path.dirname(resolvedHome);
while (currentDir && currentDir !== path.dirname(currentDir)) { while (currentDir && currentDir !== path.dirname(currentDir)) {
// Loop until filesystem root or currentDir is empty if (currentDir === path.join(resolvedHome, GEMINI_CONFIG_DIR)) {
if (debugMode) { break;
logger.debug(
`Checking for ${geminiMdFilename} in (upward scan): ${currentDir}`,
);
}
// Skip the global .gemini directory itself during upward scan from CWD,
// as global is handled separately and explicitly first.
if (currentDir === path.join(resolvedHome, GEMINI_CONFIG_DIR)) {
if (debugMode) {
logger.debug(
`Upward scan reached global config dir path, stopping upward search here: ${currentDir}`,
);
} }
break;
}
const potentialPath = path.join(currentDir, geminiMdFilename); const potentialPath = path.join(currentDir, geminiMdFilename);
try { try {
await fs.access(potentialPath, fsSync.constants.R_OK); await fs.access(potentialPath, fsSync.constants.R_OK);
// Add to upwardPaths only if it's not the already added globalMemoryPath if (potentialPath !== globalMemoryPath) {
if (potentialPath !== globalMemoryPath) { upwardPaths.unshift(potentialPath);
upwardPaths.unshift(potentialPath);
if (debugMode) {
logger.debug(
`Found readable upward ${geminiMdFilename}: ${potentialPath}`,
);
} }
} catch {
// Not found, continue.
} }
} catch {
if (debugMode) { if (currentDir === ultimateStopDir) {
logger.debug( break;
`Upward ${geminiMdFilename} not found or not readable in: ${currentDir}`,
);
} }
currentDir = path.dirname(currentDir);
} }
upwardPaths.forEach((p) => allPaths.add(p));
// Stop condition: if currentDir is the ultimateStopDir, break after this iteration. const mergedOptions = {
if (currentDir === ultimateStopDir) { ...DEFAULT_MEMORY_FILE_FILTERING_OPTIONS,
if (debugMode) ...fileFilteringOptions,
logger.debug( };
`Reached ultimate stop directory for upward scan: ${currentDir}`,
); const downwardPaths = await bfsFileSearch(resolvedCwd, {
break; fileName: geminiMdFilename,
maxDirs,
debug: debugMode,
fileService,
fileFilteringOptions: mergedOptions,
});
downwardPaths.sort();
for (const dPath of downwardPaths) {
allPaths.add(dPath);
} }
currentDir = path.dirname(currentDir);
}
upwardPaths.forEach((p) => allPaths.add(p));
// Merge options with memory defaults, with options taking precedence
const mergedOptions = {
...DEFAULT_MEMORY_FILE_FILTERING_OPTIONS,
...fileFilteringOptions,
};
const downwardPaths = await bfsFileSearch(resolvedCwd, {
fileName: geminiMdFilename,
maxDirs,
debug: debugMode,
fileService,
fileFilteringOptions: mergedOptions, // Pass merged options as fileFilter
});
downwardPaths.sort(); // Sort for consistent ordering, though hierarchy might be more complex
if (debugMode && downwardPaths.length > 0)
logger.debug(
`Found downward ${geminiMdFilename} files (sorted): ${JSON.stringify(
downwardPaths,
)}`,
);
// Add downward paths only if they haven't been included already (e.g. from upward scan)
for (const dPath of downwardPaths) {
allPaths.add(dPath);
} }
} }
// Add extension context file paths // Add extension context file paths.
for (const extensionPath of extensionContextFilePaths) { for (const extensionPath of extensionContextFilePaths) {
allPaths.add(extensionPath); allPaths.add(extensionPath);
} }