fix(core): handle EISDIR error when GEMINI.md is a directory#17840
fix(core): handle EISDIR error when GEMINI.md is a directory#17840mag123c wants to merge 1 commit intogoogle-gemini:mainfrom
Conversation
Skip directories named GEMINI.md during memory discovery instead of logging misleading warnings. This prevents EISDIR errors when a directory exists where a GEMINI.md file is expected. # Fixes google-gemini#16282
Summary of ChangesHello @mag123c, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the robustness of the memory discovery mechanism by gracefully handling a specific file system edge case. Previously, if a directory was inadvertently named Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the EISDIR error that occurs when GEMINI.md is a directory by skipping it during memory discovery. The added test case is a great addition. My review includes a suggestion to improve the accuracy of the fileCount returned by the memory discovery functions, as the current implementation can be misleading when directories are skipped. I've also suggested strengthening the new test to assert this corrected behavior.
| DEFAULT_FOLDER_TRUST, | ||
| ); | ||
|
|
||
| expect(result.memoryContent).toBe(''); |
There was a problem hiding this comment.
To make this test more robust and ensure the fileCount is accurate, please also assert that result.fileCount is 0. This verifies that skipped directories are not counted as loaded files. This change is related to the suggested fix in memoryDiscovery.ts to calculate fileCount more accurately.
expect(result.memoryContent).toBe('');
expect(result.fileCount).toBe(0);| if (errorCode === 'EISDIR') { | ||
| if (debugMode) { | ||
| logger.debug(`Skipping directory at ${filePath}`); | ||
| } | ||
| return { filePath, content: null }; |
There was a problem hiding this comment.
Returning null content here correctly handles the EISDIR error without warnings. However, this reveals an issue in loadServerHierarchicalMemory where fileCount becomes misleading. It's calculated from the total number of paths discovered, including directories that are skipped here. This means the UI might report '1 file loaded' when it was a directory and no content was actually loaded.
To ensure accuracy, fileCount should reflect the number of successfully read files. I recommend updating loadServerHierarchicalMemory to filter for non-null content before counting:
// In loadServerHierarchicalMemory...
const successfullyReadFiles = contentsWithPaths.filter(
(item) => item.content !== null
);
return {
memoryContent: combinedInstructions,
fileCount: successfullyReadFiles.length, // This is more accurate
filePaths,
};Since this is outside the current diff, I'm noting it here for you to consider as part of a complete fix.
|
Hi there! Thank you for your contribution to Gemini CLI. We really appreciate the time and effort you've put into this pull request. To keep our backlog manageable and ensure we're focusing on current priorities, we are closing pull requests that haven't seen maintainer activity for 30 days. Currently, the team is prioritizing work associated with 🔒 maintainer only or help wanted issues. If you believe this change is still critical, please feel free to comment with updated details. Otherwise, we encourage contributors to focus on open issues labeled as help wanted. Thank you for your understanding! |
1 participant
Summary
Skip directories named GEMINI.md during memory discovery instead of logging misleading warnings, preventing EISDIR errors.
Details
readGeminiMdFiles()catch blockRelated Issues
Fixes #16282
How to Validate
GEMINI.mdin a project root.npm start/aboutor any command that triggers memory discovery.Pre-Merge Checklist