fix(cli): prevent multiple banner increments on remount (#24843)

This commit is contained in:
Sehoon Shon
2026-04-07 15:44:09 -04:00
committed by GitHub
parent ab3075feb9
commit d29da15427
4 changed files with 36 additions and 23 deletions

View File

@@ -10,15 +10,20 @@ import {
} from '../../test-utils/render.js';
import type { LoadedSettings } from '../../config/settings.js';
import { AppHeader } from './AppHeader.js';
import { describe, it, expect, vi } from 'vitest';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { makeFakeConfig } from '@google/gemini-cli-core';
import crypto from 'node:crypto';
import { _clearSessionBannersForTest } from '../hooks/useBanner.js';
vi.mock('../utils/terminalSetup.js', () => ({
getTerminalProgram: () => null,
}));
describe('<AppHeader />', () => {
beforeEach(() => {
_clearSessionBannersForTest();
});
it('should render the banner with default text', async () => {
const uiState = {
history: [],

View File

@@ -168,13 +168,6 @@ exports[`InputPrompt > mouse interaction > should toggle paste expansion on doub
"
`;
exports[`InputPrompt > mouse interaction > should toggle paste expansion on double-click 4`] = `
"▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
> [Pasted Text: 10 lines]
▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄
"
`;
exports[`InputPrompt > multiline rendering > should correctly render multiline input including blank lines 1`] = `
"────────────────────────────────────────────────────────────────────────────────────────────────────
│ > hello │

View File

@@ -13,7 +13,7 @@ import {
type MockedFunction,
} from 'vitest';
import { renderHook } from '../../test-utils/render.js';
import { useBanner } from './useBanner.js';
import { useBanner, _clearSessionBannersForTest } from './useBanner.js';
import { persistentState } from '../../utils/persistentState.js';
import crypto from 'node:crypto';
@@ -56,6 +56,7 @@ describe('useBanner', () => {
beforeEach(() => {
vi.resetAllMocks();
_clearSessionBannersForTest();
// Default persistentState behavior: return empty object (no counts)
mockedPersistentStateGet.mockReturnValue({});
@@ -101,13 +102,18 @@ describe('useBanner', () => {
);
});
it('should NOT increment count if warning text is shown instead', async () => {
it('should increment count if warning text is shown instead', async () => {
const data = { defaultText: 'Standard', warningText: 'Warning' };
await renderHook(() => useBanner(data));
// Since warning text takes precedence, default banner logic (and increment) is skipped
expect(mockedPersistentStateSet).not.toHaveBeenCalled();
// Warning text now also gets counted
expect(mockedPersistentStateSet).toHaveBeenCalledWith(
'defaultBannerShownCount',
{
[crypto.createHash('sha256').update(data.warningText).digest('hex')]: 1,
},
);
});
it('should handle newline replacements', async () => {

View File

@@ -4,12 +4,21 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { useState, useEffect, useRef } from 'react';
import { useState, useEffect } from 'react';
import { persistentState } from '../../utils/persistentState.js';
import crypto from 'node:crypto';
const DEFAULT_MAX_BANNER_SHOWN_COUNT = 5;
// Track banners incremented during this session to prevent multiple increments
// on React unmounts/remounts
const sessionIncrementedBanners = new Set<string>();
// For testing purposes
export function _clearSessionBannersForTest() {
sessionIncrementedBanners.clear();
}
interface BannerData {
defaultText: string;
warningText: string;
@@ -22,25 +31,25 @@ export function useBanner(bannerData: BannerData) {
() => persistentState.get('defaultBannerShownCount') || {},
);
const activeText = warningText ? warningText : defaultText;
const hashedText = crypto
.createHash('sha256')
.update(defaultText)
.update(activeText)
.digest('hex');
const currentBannerCount = bannerCounts[hashedText] || 0;
const showDefaultBanner =
warningText === '' && currentBannerCount < DEFAULT_MAX_BANNER_SHOWN_COUNT;
const showBanner =
activeText !== '' && currentBannerCount < DEFAULT_MAX_BANNER_SHOWN_COUNT;
const rawBannerText = showDefaultBanner ? defaultText : warningText;
const rawBannerText = showBanner ? activeText : '';
const bannerText = rawBannerText.replace(/\\n/g, '\n');
const lastIncrementedKey = useRef<string | null>(null);
useEffect(() => {
if (showDefaultBanner && defaultText) {
if (lastIncrementedKey.current !== defaultText) {
lastIncrementedKey.current = defaultText;
if (showBanner && activeText) {
if (!sessionIncrementedBanners.has(activeText)) {
sessionIncrementedBanners.add(activeText);
const allCounts = persistentState.get('defaultBannerShownCount') || {};
const current = allCounts[hashedText] || 0;
@@ -51,7 +60,7 @@ export function useBanner(bannerData: BannerData) {
});
}
}
}, [showDefaultBanner, defaultText, hashedText]);
}, [showBanner, activeText, hashedText]);
return {
bannerText,