fix(cli): use os.homedir() for home directory warning check#25890
fix(cli): use os.homedir() for home directory warning check#25890TirthNaik-99 wants to merge 5 commits intogoogle-gemini:mainfrom
Conversation
The home directory warning incorrectly used the core homedir() helper which respects the GEMINI_CLI_HOME environment variable. When GEMINI_CLI_HOME is set to a non-home directory, the warning could fire in subdirectories or miss the actual home directory entirely. Switch to Node's native os.homedir() so the check always compares against the real OS home directory, and normalize both resolved paths before comparison to handle trailing-slash and separator edge cases. Add test coverage for subdirectories, symlinked home directories, and GEMINI_CLI_HOME override scenarios. Fixes google-gemini#22309
Summary of ChangesHello, 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 addresses an issue where the Gemini CLI incorrectly triggers a home directory warning when running in subdirectories. By switching to the native OS home directory resolution and adding path normalization, the CLI now accurately distinguishes between the actual home directory and its subdirectories, improving the user experience and preventing unnecessary warnings. 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request updates the home directory detection logic in userStartupWarnings.ts by switching to node:os and applying path normalization to handle symlinks and subdirectories correctly. It also adds several test cases to verify these scenarios. One review comment identifies a violation of the repository's testing style guide, specifically recommending the use of vi.stubEnv() instead of direct process.env modification to prevent test leakage.
| await fs.mkdir(projectDir, { recursive: true }); | ||
| const originalEnv = process.env['GEMINI_CLI_HOME']; | ||
| process.env['GEMINI_CLI_HOME'] = projectDir; | ||
|
|
||
| const warnings = await getUserStartupWarnings({}, projectDir); | ||
| expect(warnings.find((w) => w.id === 'home-directory')).toBeUndefined(); | ||
|
|
||
| if (originalEnv === undefined) { | ||
| delete process.env['GEMINI_CLI_HOME']; | ||
| } else { | ||
| process.env['GEMINI_CLI_HOME'] = originalEnv; |
There was a problem hiding this comment.
Directly modifying process.env is discouraged in this repository as it can lead to test leakage and is less reliable than using built-in Vitest utilities. Per the repository style guide, you should use vi.stubEnv() to set environment variables. Please also ensure that vi.unstubAllEnvs() is added to the afterEach block (around line 65) to properly clean up the environment after each test.
| await fs.mkdir(projectDir, { recursive: true }); | |
| const originalEnv = process.env['GEMINI_CLI_HOME']; | |
| process.env['GEMINI_CLI_HOME'] = projectDir; | |
| const warnings = await getUserStartupWarnings({}, projectDir); | |
| expect(warnings.find((w) => w.id === 'home-directory')).toBeUndefined(); | |
| if (originalEnv === undefined) { | |
| delete process.env['GEMINI_CLI_HOME']; | |
| } else { | |
| process.env['GEMINI_CLI_HOME'] = originalEnv; | |
| vi.stubEnv('GEMINI_CLI_HOME', projectDir); | |
| const warnings = await getUserStartupWarnings({}, projectDir); | |
| expect(warnings.find((w) => w.id === 'home-directory')).toBeUndefined(); |
References
- When testing code that depends on environment variables, use
vi.stubEnv('NAME', 'value')inbeforeEachandvi.unstubAllEnvs()inafterEach. Avoid modifyingprocess.envdirectly as it can lead to test leakage and is less reliable. (link)
…tion Address review feedback: use vi.stubEnv() for environment variable manipulation in tests to prevent test leakage, per repository style guide.
The home directory warning incorrectly used the core homedir() helper
which respects the GEMINI_CLI_HOME environment variable. When
GEMINI_CLI_HOME is set to a non-home directory, the warning could fire
in subdirectories or miss the actual home directory entirely.
Switch to Node's native os.homedir() so the check always compares
against the real OS home directory, and normalize both resolved paths
before comparison to handle trailing-slash and separator edge cases.
Add test coverage for subdirectories, symlinked home directories, and
GEMINI_CLI_HOME override scenarios.
Fixes #22309
Summary
Fix home directory warning triggering incorrectly in subdirectories by using
os.homedir()instead of the corehomedir()helper, and normalizing paths before comparison.Details
homedir()from@google/gemini-cli-corerespects theGEMINI_CLI_HOMEenv var, which is intended for config directory resolution — not for checking if the user is literally in their OS home directory. Replaced with Node's nativeos.homedir().path.normalize()on bothfs.realpath()outputs before the===comparison to handle trailing-slash and separator edge cases across platforms.GEMINI_CLI_HOMEoverride scenario.Related Issues
Fixes #22309
How to Validate
npx vitest run packages/cli/src/utils/userStartupWarnings.test.ts- All 13 tests should pass.npx tsc --noEmit -p packages/cli/tsconfig.jsonnpm run buildgeminifrom your home directory (cd ~) — warning should appeargeminifrom a subdirectory (cd ~/some-project) — warning should NOT appearPre-Merge Checklist