fix(cli): use tmux-safe thinking indicator#22067
fix(cli): use tmux-safe thinking indicator#22067apfine wants to merge 6 commits intogoogle-gemini:mainfrom
Conversation
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 terminal redraw interference experienced when the Gemini CLI's loading spinner runs inside a tmux pane by introducing a tmux-safe, low-frequency indicator. Additionally, it enhances the Highlights
Changelog
Activity
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
|
There was a problem hiding this comment.
Code Review
This pull request introduces two valuable but unrelated changes. The first is a user experience improvement for tmux users, which correctly addresses the issue of terminal redraw interference by using a low-frequency indicator. The implementation is clean and effective. The second change is a critical security fix and UX enhancement for the Antigravity IDE installer. It correctly sanitizes the ANTIGRAVITY_CLI_ALIAS environment variable to prevent command injection vulnerabilities and adds helpful fallbacks for command detection. While both changes are positive, they should ideally be in separate, focused pull requests as per the project's contribution guidelines (Keep PRs small, focused...). This improves reviewability and makes the commit history easier to follow. For instance, the important security fix is not mentioned in the PR title or description, which could cause it to be overlooked.
|
@Adib234 sir please could you review this ? according to gemini code assist bot there were some issues so I resolved them . It would be very helpful sir !! |
| const TMUX_UPDATE_INTERVAL_MS = 750; | ||
| const TMUX_FRAMES = ['.', '..', '...'] as const; | ||
|
|
||
| function isTmuxEnvironment(): boolean { |
There was a problem hiding this comment.
The isTmuxEnvironment() function duplicates existing logic. We already have an inTmux() function in packages/cli/src/ui/utils/commandUtils.ts. Could you please export inTmux from that file and use it in GeminiSpinner.tsx instead of duplicating the logic?
spencer426
left a comment
There was a problem hiding this comment.
The PR description mentions adding focused regression tests for the new behavior GeminiSpinner.test.tsx, but the test file is not included in the actual PR diff. It looks like it might have been missed during the commit/push process. Could you please add it?
Sure !! |
|
@spencer426 I have done the suggested changes. Please review it . Thankyou so much for your valuable time !! |
| beforeEach(() => { | ||
| vi.unstubAllEnvs(); | ||
| mockUseIsScreenReaderEnabled.mockReturnValue(false); | ||
| }); |
There was a problem hiding this comment.
To prevent test flakiness, explicitly unset the TMUX and TERM environment variables here. If someone runs npm test inside tmux, inTmux() will evaluate to true and cause the non-tmux test to fail.
vi.stubEnv('TMUX', '');
vi.stubEnv('TERM', '');| afterEach(() => { | ||
| vi.useRealTimers(); | ||
| vi.unstubAllEnvs(); | ||
| }); |
There was a problem hiding this comment.
Per the Core Testing Guidelines, please add vi.restoreAllMocks() here to prevent test pollution.
| ); | ||
|
|
||
| await waitUntilReady(); | ||
| expect(lastFrame()).toContain('.'); |
There was a problem hiding this comment.
The React Testing Guidelines specify: "Use toMatchSnapshot to verify that rendering works as expected rather than matching against the raw content of the output." Please update these assertions to use toMatchSnapshot() for each frame state.
|
@spencer426 I have did the changes . Please review them !! |
|
@spencer426 |
|
Hi, I would like to work on this issue. I’m a GSoC 2026 applicant and have experience with Node.js and CLI tools. I’m particularly interested in improving CLI behavior and terminal compatibility. Could you please guide me on how to proceed? Thanks! |
|
Apologies, I mistakenly commented on a PR instead of an issue. Thank you for the clarification. I’ll look for an appropriate open issue to contribute to. |
| it('renders a tmux-safe fixed-width dots indicator when in tmux', async () => { | ||
| vi.useFakeTimers(); | ||
| vi.stubEnv('TMUX', '/tmp/tmux-1000/default,123,0'); | ||
| const { lastFrame, waitUntilReady, unmount } = renderWithProviders( | ||
| <GeminiSpinner />, | ||
| ); | ||
|
|
||
| await waitUntilReady(); | ||
| expect(lastFrame()).toMatchSnapshot(); | ||
|
|
||
| await act(async () => { | ||
| await vi.advanceTimersByTimeAsync(750); | ||
| }); | ||
| await waitUntilReady(); | ||
| expect(lastFrame()).toMatchSnapshot(); | ||
|
|
||
| await act(async () => { | ||
| await vi.advanceTimersByTimeAsync(750); | ||
| }); | ||
| await waitUntilReady(); | ||
| expect(lastFrame()).toMatchSnapshot(); | ||
|
|
There was a problem hiding this comment.
Here, the tmux dots animation test advances through all three frames (. → .. → ...) but stops there. It never tests what happens on the next tick, which is the most important one: that's where the modulo logic in GeminiSpinner.tsx (lines 55-57) wraps the index from 2 back to 0 so the animation loops.
If someone accidentally removed the % TMUX_FRAMES.length from GeminiSpinner.tsx (line 56), the index would keep climbing: 0 → 1 → 2 → 3. Index 3 doesn't exist in a 3-element array, so the animation would break. But the current test stops after reaching index 2, so it would still pass and never catch that problem.
Adding one more timer advance before the unmount() at line 74 would cover this:
await act(async () => {
await vi.advanceTimersByTimeAsync(750);
});
await waitUntilReady();
// Verify wrap-around: should cycle back to '.'
expect(lastFrame()).toMatchSnapshot();There was a problem hiding this comment.
sure , I am looking in to it .
Thankyou I will revert back soon with fixes.
Description
This change fixes terminal redraw interference when Gemini CLI is running inside a tmux pane.
While the CLI is thinking, the loading spinner redraws frequently. In tmux split-pane setups, those terminal control sequence updates can interfere with mouse text selection in neighboring panes, causing highlighted text to disappear while Gemini is still responding.
The goal of this change is to preserve visible progress feedback while reducing the redraw behavior that disrupts adjacent tmux panes.
Changes Made
......750msTesting
Results:
GeminiSpinner.test.tsx: passed (3/3)
spinner/loading component suite: passed (28/28)
targeted ESLint on touched files: passed
Related Issues
Fixes #19675