mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-13 05:12:55 -07:00
fix(cli): prevent duplicate SessionStart systemMessage render (#25827)
Co-authored-by: Jacob Richman <jacob314@gmail.com>
This commit is contained in:
@@ -1267,6 +1267,42 @@ describe('AppContainer State Management', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('SessionStart Hook Rendering', () => {
|
||||
it('does not render systemMessage directly (avoids duplicate with HookSystemMessage event)', async () => {
|
||||
const mockAddItem = vi.fn();
|
||||
mockedUseHistory.mockReturnValue({
|
||||
history: [],
|
||||
addItem: mockAddItem,
|
||||
updateItem: vi.fn(),
|
||||
clearItems: vi.fn(),
|
||||
loadHistory: vi.fn(),
|
||||
});
|
||||
|
||||
const fireSessionStartEvent = vi.fn().mockResolvedValue({
|
||||
systemMessage: 'Hello from SessionStart hook',
|
||||
getAdditionalContext: vi.fn(() => undefined),
|
||||
});
|
||||
vi.spyOn(mockConfig, 'getHookSystem').mockReturnValue({
|
||||
fireSessionEndEvent: vi.fn().mockResolvedValue(undefined),
|
||||
fireSessionStartEvent,
|
||||
} as unknown as ReturnType<Config['getHookSystem']>);
|
||||
|
||||
const { unmount } = await act(async () => renderAppContainer());
|
||||
await waitFor(() => expect(fireSessionStartEvent).toHaveBeenCalled());
|
||||
|
||||
// The direct-render path (the bug) would call addItem with the
|
||||
// systemMessage text and no `source` field. The HookSystemMessage
|
||||
// event-listener path (the correct one) always sets `source`.
|
||||
const directRenderCall = mockAddItem.mock.calls.find(
|
||||
([item]) =>
|
||||
item?.text === 'Hello from SessionStart hook' && !item?.source,
|
||||
);
|
||||
expect(directRenderCall).toBeUndefined();
|
||||
|
||||
unmount();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Token Counting from Session Stats', () => {
|
||||
it('tracks token counts from session messages', async () => {
|
||||
// Session stats are provided through the SessionStatsProvider context
|
||||
|
||||
@@ -497,16 +497,6 @@ export const AppContainer = (props: AppContainerProps) => {
|
||||
?.fireSessionStartEvent(sessionStartSource);
|
||||
|
||||
if (result) {
|
||||
if (result.systemMessage) {
|
||||
historyManager.addItem(
|
||||
{
|
||||
type: MessageType.INFO,
|
||||
text: result.systemMessage,
|
||||
},
|
||||
Date.now(),
|
||||
);
|
||||
}
|
||||
|
||||
const additionalContext = result.getAdditionalContext();
|
||||
const geminiClient = config.getGeminiClient();
|
||||
if (additionalContext && geminiClient) {
|
||||
@@ -549,12 +539,6 @@ export const AppContainer = (props: AppContainerProps) => {
|
||||
debugLogger.error('Error during cleanup:', e),
|
||||
);
|
||||
};
|
||||
// Disable the dependencies check here. historyManager gets flagged
|
||||
// but we don't want to react to changes to it because each new history
|
||||
// item, including the ones from the start session hook will cause a
|
||||
// re-render and an error when we try to reload config.
|
||||
//
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [config, resumedSessionData]);
|
||||
|
||||
useEffect(
|
||||
|
||||
@@ -36,6 +36,7 @@ const mockCoreEvents = vi.hoisted(() => ({
|
||||
emitFeedback: vi.fn(),
|
||||
emitHookStart: vi.fn(),
|
||||
emitHookEnd: vi.fn(),
|
||||
emitHookSystemMessage: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../utils/debugLogger.js', () => ({
|
||||
@@ -891,4 +892,100 @@ describe('HookEventHandler', () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('systemMessage event emission', () => {
|
||||
const buildMocks = (
|
||||
outputFormat: 'json' | 'text',
|
||||
systemMessage: string,
|
||||
) => {
|
||||
const hookConfig: HookConfig = {
|
||||
type: HookType.Command,
|
||||
command: './hook.sh',
|
||||
timeout: 30000,
|
||||
};
|
||||
const results: HookExecutionResult[] = [
|
||||
{
|
||||
success: true,
|
||||
duration: 10,
|
||||
hookConfig,
|
||||
eventName: HookEventName.SessionStart,
|
||||
output: { systemMessage },
|
||||
outputFormat,
|
||||
},
|
||||
];
|
||||
vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue({
|
||||
eventName: HookEventName.SessionStart,
|
||||
hookConfigs: [hookConfig],
|
||||
sequential: false,
|
||||
});
|
||||
vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue(results);
|
||||
vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue({
|
||||
success: true,
|
||||
allOutputs: [],
|
||||
errors: [],
|
||||
totalDuration: 10,
|
||||
});
|
||||
};
|
||||
|
||||
it('emits HookSystemMessage for json-format hook output', async () => {
|
||||
buildMocks('json', 'json banner');
|
||||
|
||||
await hookEventHandler.fireSessionStartEvent(SessionStartSource.Startup);
|
||||
|
||||
expect(mockCoreEvents.emitHookSystemMessage).toHaveBeenCalledTimes(1);
|
||||
expect(mockCoreEvents.emitHookSystemMessage).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
eventName: HookEventName.SessionStart,
|
||||
message: 'json banner',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('emits HookSystemMessage for text-format hook output', async () => {
|
||||
buildMocks('text', 'plain-text banner');
|
||||
|
||||
await hookEventHandler.fireSessionStartEvent(SessionStartSource.Startup);
|
||||
|
||||
expect(mockCoreEvents.emitHookSystemMessage).toHaveBeenCalledTimes(1);
|
||||
expect(mockCoreEvents.emitHookSystemMessage).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
eventName: HookEventName.SessionStart,
|
||||
message: 'plain-text banner',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('does not emit when systemMessage is absent', async () => {
|
||||
const hookConfig: HookConfig = {
|
||||
type: HookType.Command,
|
||||
command: './hook.sh',
|
||||
timeout: 30000,
|
||||
};
|
||||
vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue({
|
||||
eventName: HookEventName.SessionStart,
|
||||
hookConfigs: [hookConfig],
|
||||
sequential: false,
|
||||
});
|
||||
vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([
|
||||
{
|
||||
success: true,
|
||||
duration: 10,
|
||||
hookConfig,
|
||||
eventName: HookEventName.SessionStart,
|
||||
output: {},
|
||||
outputFormat: 'json',
|
||||
},
|
||||
]);
|
||||
vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue({
|
||||
success: true,
|
||||
allOutputs: [],
|
||||
errors: [],
|
||||
totalDuration: 10,
|
||||
});
|
||||
|
||||
await hookEventHandler.fireSessionStartEvent(SessionStartSource.Startup);
|
||||
|
||||
expect(mockCoreEvents.emitHookSystemMessage).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -459,8 +459,9 @@ export class HookEventHandler {
|
||||
|
||||
logHookCall(this.context.config, hookCallEvent);
|
||||
|
||||
// Emit structured system message event for UI display
|
||||
if (result.output?.systemMessage && result.outputFormat === 'json') {
|
||||
// Emit structured system message event for UI display. Covers both
|
||||
// 'json' and 'text' output formats so plain-text hook stdout also surfaces.
|
||||
if (result.output?.systemMessage) {
|
||||
coreEvents.emitHookSystemMessage({
|
||||
hookName,
|
||||
eventName,
|
||||
|
||||
Reference in New Issue
Block a user