feat(cli): add hostname to footer with smart visibility#25730
feat(cli): add hostname to footer with smart visibility#25730NTaylorMullen wants to merge 1 commit intomainfrom
Conversation
- Add hostname to footer ALL_ITEMS and DEFAULT_ORDER - Implement smart visibility for hostname (only show when remote/container) - Shorten hostname to first segment to save space - Fix vimEnabled destructuring in Footer.tsx Fixes #25661
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 enhances the Gemini CLI user interface by introducing a context-aware hostname display in the footer. By detecting remote or containerized execution environments, the CLI now provides helpful infrastructure context to users working across multiple sessions while maintaining a clean UI for local users. Additionally, minor bug fixes and configuration updates were included to ensure the new feature is discoverable and correctly integrated into the existing settings framework. 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 adds a 'hostname' item to the CLI footer, displayed when the environment is remote or containerized. The update includes configuration changes, logic to detect remote sessions via environment variables and Docker presence, and corresponding unit tests. Feedback was provided to optimize performance by moving the synchronous filesystem check for Docker detection to the module level, preventing redundant I/O operations during component re-renders.
| } from '../../config/footerItems.js'; | ||
| import { isDevelopment } from '../../utils/installationInfo.js'; | ||
|
|
||
| const SYSTEM_HOSTNAME = hostname().split('.')[0]; |
There was a problem hiding this comment.
The Docker environment check should be performed once at the module level to avoid repeated synchronous I/O during component renders. Synchronous filesystem operations like fs.existsSync inside a React component's render cycle are a performance anti-pattern, as they execute on every re-render.
| const SYSTEM_HOSTNAME = hostname().split('.')[0]; | |
| const SYSTEM_HOSTNAME = hostname().split('.')[0]; | |
| const IS_DOCKER = process.platform === 'linux' && fs.existsSync('/.dockerenv'); |
| const isRemote = | ||
| process.env['SSH_TTY'] || | ||
| process.env['SSH_CONNECTION'] || | ||
| process.env['SSH_CLIENT'] || | ||
| process.env['CLOUD_SHELL'] === 'true' || | ||
| process.env['EDITOR_IN_CLOUD_SHELL'] === 'true' || | ||
| process.env['KUBERNETES_SERVICE_HOST'] || | ||
| (process.platform === 'linux' && fs.existsSync('/.dockerenv')); |
There was a problem hiding this comment.
As noted above, the isRemote check should use a module-level constant for the Docker check to avoid repeated blocking I/O. The environment variable checks are kept inside the component to ensure they remain testable with vi.stubEnv.
| const isRemote = | |
| process.env['SSH_TTY'] || | |
| process.env['SSH_CONNECTION'] || | |
| process.env['SSH_CLIENT'] || | |
| process.env['CLOUD_SHELL'] === 'true' || | |
| process.env['EDITOR_IN_CLOUD_SHELL'] === 'true' || | |
| process.env['KUBERNETES_SERVICE_HOST'] || | |
| (process.platform === 'linux' && fs.existsSync('/.dockerenv')); | |
| const isRemote = !!( | |
| process.env['SSH_TTY'] || | |
| process.env['SSH_CONNECTION'] || | |
| process.env['SSH_CLIENT'] || | |
| process.env['CLOUD_SHELL'] === 'true' || | |
| process.env['EDITOR_IN_CLOUD_SHELL'] === 'true' || | |
| process.env['KUBERNETES_SERVICE_HOST'] || | |
| IS_DOCKER | |
| ); |
|
Size Change: +912 B (0%) Total Size: 33.7 MB
ℹ️ View Unchanged
|
nidhishgajjar
left a comment
There was a problem hiding this comment.
Review of PR #25730: feat(cli): add hostname to footer with smart visibility
Summary
This PR adds system hostname display to the Gemini CLI footer with intelligent visibility - only showing when running in remote or containerized environments (SSH, Cloud Shell, Docker, Kubernetes).
Changes Analysis
1. packages/cli/src/config/footerItems.ts ✅
Added hostname configuration:
- Added to
ALL_ITEMSwith idhostname, headerhost, and clear description - Added to
DEFAULT_ORDERbetweensandboxandmodel-name(logical placement) - Added to
deriveItemsFromLegacySettingsfor backward compatibility
Placement is appropriate - hostname appears after sandbox info but before model name, making it easy to spot in remote sessions.
2. packages/cli/src/ui/components/Footer.test.tsx ✅
Comprehensive test coverage:
- Test 1: Verifies hostname renders when remote (
SSH_TTYset) - Test 2: Verifies hostname does NOT render when not remote (clean env)
Test quality:
- Uses
vi.stubEnvfor environment variable mocking - Tests both positive and negative scenarios
- Properly cleans up with
unmount() - Uses
allowEmpty: truefor the negative test
3. packages/cli/src/ui/components/Footer.tsx ✅
Hostname extraction:
const SYSTEM_HOSTNAME = hostname().split(".")[0];- Truncates to first segment (e.g.,
my-serverinstead ofmy-server.internal.domain.com) - Preserves horizontal space in the footer
- Good UX decision
Remote detection:
const isRemote =
process.env["SSH_TTY"] ||
process.env["SSH_CONNECTION"] ||
process.env["SSH_CLIENT"] ||
process.env["CLOUD_SHELL"] === "true" ||
process.env["EDITOR_IN_CLOUD_SHELL"] === "true" ||
process.env["KUBERNETES_SERVICE_HOST"] ||
(process.platform === "linux" && fs.existsSync("/.dockerenv"));Comprehensive coverage:
- SSH:
SSH_TTY,SSH_CONNECTION,SSH_CLIENT - Google Cloud Shell:
CLOUD_SHELL,EDITOR_IN_CLOUD_SHELL - Kubernetes:
KUBERNETES_SERVICE_HOST - Docker: Checks for
/.dockerenvon Linux
Smart visibility logic:
case "hostname": {
if (isRemote && SYSTEM_HOSTNAME) {
addCol(
id,
header,
() => <Text color={itemColor}>{SYSTEM_HOSTNAME}</Text>,
SYSTEM_HOSTNAME.length,
);
}
break;
}- Only renders when both conditions are met
- Prevents display for local sessions
- Handles empty hostname gracefully
Bug fix (bonus):
// Before:
const { vimEnabled, vimMode } = useVimMode();
// After:
const { vimMode, vimEnabled } = useVimMode();- Fixed destructuring order
- This was likely causing issues with vim mode detection
Strengths
- Smart visibility: Only shows hostname when relevant (remote/container)
- Comprehensive detection: Covers SSH, Cloud Shell, Kubernetes, Docker
- Good UX: Truncated hostname saves space
- Well-tested: Both positive and negative scenarios covered
- Bug fix included: Fixed vim mode destructuring
- Backward compatible: Added to legacy settings migration
- Good documentation: Clear PR description with validation steps
Minor Suggestions
1. Type Safety
The isRemote check relies on truthy values, which could be more explicit:
const isRemote =
!!process.env["SSH_TTY"] ||
!!process.env["SSH_CONNECTION"] ||
!!process.env["SSH_CLIENT"] ||
process.env["CLOUD_SHELL"] === "true" ||
process.env["EDITOR_IN_CLOUD_SHELL"] === "true" ||
!!process.env["KUBERNETES_SERVICE_HOST"] ||
(process.platform === "linux" && fs.existsSync("/.dockerenv"));However, the current implementation works fine since these env vars are either set or not set.
2. Docker Detection on non-Linux
The Docker detection only works on Linux:
(process.platform === "linux" && fs.existsSync("/.dockerenv"))Consider expanding to other platforms:
const isDocker =
(process.platform === "linux" && fs.existsSync("/.dockerenv")) ||
(process.platform === "darwin" && /* macOS Docker detection */) ||
(process.platform === "win32" && /* Windows Docker detection */);However, this may be acceptable if Docker is primarily used on Linux for this use case.
3. Hostname Truncation Edge Case
The hostname truncation could result in an empty string:
const SYSTEM_HOSTNAME = hostname().split(".")[0];If hostname() returns .example.com, this would result in an empty string. Consider:
const SYSTEM_HOSTNAME = hostname().split(".")[0] || hostname();However, this is unlikely in practice.
4. Platform-Specific Tests
The pre-merge checklist shows:
- MacOS (npm run, npx)
- Windows
- Linux
- Docker/Podman/Seatbelt
Consider adding tests for:
- Windows remote detection
- Docker environment detection
- Cloud Shell environment
Potential Concerns
1. False Positives
The isRemote detection could have false positives:
SSH_TTYmight be set in some local development setupsKUBERNETES_SERVICE_HOSTcould be set in local Kubernetes clusters
However, showing the hostname in these cases is probably acceptable - it provides context without being harmful.
2. Performance
The fs.existsSync("/.dockerenv") check is synchronous. For better performance, consider:
import { existsSync } from "node:fs";
// ...
const isDocker = process.platform === "linux" && existsSync("/.dockerenv");However, this is called once per render, and the performance impact is negligible.
Approval Criteria
I would approve this PR if:
- Changes are well-structured and maintainable ✅
- Tests cover the new behavior ✅
- Smart visibility logic is comprehensive ✅
- Bug fix included (vim mode destructuring) ✅
- Good documentation ✅
- Platform-specific tests completed (from pre-merge checklist)
Overall Assessment
✅ Excellent implementation with smart visibility
✅ Comprehensive remote/container detection
✅ Well-tested with both scenarios
✅ Includes bug fix (vim mode destructuring)
✅ Good UX decisions (truncated hostname)
This is a well-implemented feature that addresses a real user need (identifying remote sessions) without cluttering the UI for local users. The smart visibility logic is comprehensive and the tests are thorough.
Recommendation: Approve (pending completion of pre-merge checklist items)
|
I think you meant this PR? @jackwotherspoon |
Summary
Add the system hostname to the persistent bottom status border of the Gemini CLI when running in remote or containerized environments.
Details
my-serverinstead ofmy-server.internal.domain.com) to preserve horizontal space.vimEnabledwas incorrectly destructured asenabled.Related Issues
Fixes #25571
How to Validate
npm run start. Verify the hostname does NOT appear in the footer.SSH_TTY=/dev/pts/1 npm run start. Verify the hostname appears in the footer (usually between the sandbox and model name)./settings-> UI -> Footer Items) and verifyhostnameis available for reordering or toggling.Pre-Merge Checklist