Skip to content

Commit 1689e9b

Browse files
authored
fix(cli): fix issue updating a component while rendering a different component (#14319)
1 parent 98d7238 commit 1689e9b

File tree

9 files changed

+43
-90
lines changed

9 files changed

+43
-90
lines changed

integration-tests/extensions-reload.test.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,16 @@ describe('extension reloading', () => {
7676
await run.expectText(
7777
'test-extension (v0.0.1) - active (update available)',
7878
);
79-
await run.sendText('/mcp list');
80-
await run.type('\r');
79+
// Wait a bit for the UI to settle
80+
await new Promise((resolve) => setTimeout(resolve, 1000));
81+
await run.sendKeys('\u0015/mcp list\r');
8182
await run.expectText(
8283
'test-server (from test-extension) - Ready (1 tool)',
8384
);
8485
await run.expectText('- hello');
8586

8687
// Update the extension, expect the list to update, and mcp servers as well.
87-
await run.sendKeys('/extensions update test-extension');
88+
await run.sendKeys('\u0015/extensions update test-extension');
8889
await run.expectText('/extensions update test-extension');
8990
await run.sendKeys('\r');
9091
await new Promise((resolve) => setTimeout(resolve, 500));
@@ -96,12 +97,12 @@ describe('extension reloading', () => {
9697
await run.expectText(
9798
'Extension "test-extension" successfully updated: 0.0.1 → 0.0.2',
9899
);
99-
await new Promise((resolve) => setTimeout(resolve, 1000));
100-
await run.sendText('/extensions list');
101-
await run.type('\r');
100+
await new Promise((resolve) => setTimeout(resolve, 3000));
101+
await run.sendKeys('\u0015/extensions list\r');
102102
await run.expectText('test-extension (v0.0.2) - active (updated)');
103-
await run.sendText('/mcp list');
104-
await run.type('\r');
103+
// Wait a bit for the UI to settle
104+
await new Promise((resolve) => setTimeout(resolve, 1000));
105+
await run.sendKeys('\u0015/mcp list\r');
105106
await run.expectText(
106107
'test-server (from test-extension) - Ready (1 tool)',
107108
);

packages/cli/src/ui/AppContainer.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -832,11 +832,14 @@ Logging in with Google... Restarting Gemini CLI to continue.
832832
useLayoutEffect(() => {
833833
if (mainControlsRef.current) {
834834
const fullFooterMeasurement = measureElement(mainControlsRef.current);
835-
if (fullFooterMeasurement.height > 0) {
835+
if (
836+
fullFooterMeasurement.height > 0 &&
837+
fullFooterMeasurement.height !== controlsHeight
838+
) {
836839
setControlsHeight(fullFooterMeasurement.height);
837840
}
838841
}
839-
}, [buffer, terminalWidth, terminalHeight]);
842+
}, [buffer, terminalWidth, terminalHeight, controlsHeight]);
840843

841844
// Compute available terminal height based on controls measurement
842845
const availableTerminalHeight = Math.max(

packages/cli/src/ui/components/InputPrompt.test.tsx

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1147,7 +1147,6 @@ describe('InputPrompt', () => {
11471147
await waitFor(() => {
11481148
expect(mockedUseCommandCompletion).toHaveBeenCalledWith(
11491149
mockBuffer,
1150-
['/test/project/src'],
11511150
path.join('test', 'project', 'src'),
11521151
mockSlashCommands,
11531152
mockCommandContext,
@@ -2268,6 +2267,7 @@ describe('InputPrompt', () => {
22682267
describe('queued message editing', () => {
22692268
it('should load all queued messages when up arrow is pressed with empty input', async () => {
22702269
const mockPopAllMessages = vi.fn();
2270+
mockPopAllMessages.mockReturnValue('Message 1\n\nMessage 2\n\nMessage 3');
22712271
props.popAllMessages = mockPopAllMessages;
22722272
props.buffer.text = '';
22732273

@@ -2279,11 +2279,7 @@ describe('InputPrompt', () => {
22792279
stdin.write('\u001B[A');
22802280
});
22812281
await waitFor(() => expect(mockPopAllMessages).toHaveBeenCalled());
2282-
const callback = mockPopAllMessages.mock.calls[0][0];
22832282

2284-
await act(async () => {
2285-
callback('Message 1\n\nMessage 2\n\nMessage 3');
2286-
});
22872283
expect(props.buffer.setText).toHaveBeenCalledWith(
22882284
'Message 1\n\nMessage 2\n\nMessage 3',
22892285
);
@@ -2311,6 +2307,7 @@ describe('InputPrompt', () => {
23112307

23122308
it('should handle undefined messages from popAllMessages', async () => {
23132309
const mockPopAllMessages = vi.fn();
2310+
mockPopAllMessages.mockReturnValue(undefined);
23142311
props.popAllMessages = mockPopAllMessages;
23152312
props.buffer.text = '';
23162313

@@ -2322,10 +2319,6 @@ describe('InputPrompt', () => {
23222319
stdin.write('\u001B[A');
23232320
});
23242321
await waitFor(() => expect(mockPopAllMessages).toHaveBeenCalled());
2325-
const callback = mockPopAllMessages.mock.calls[0][0];
2326-
await act(async () => {
2327-
callback(undefined);
2328-
});
23292322

23302323
expect(props.buffer.setText).not.toHaveBeenCalled();
23312324
expect(mockInputHistory.navigateUp).toHaveBeenCalled();
@@ -2353,6 +2346,7 @@ describe('InputPrompt', () => {
23532346

23542347
it('should handle single queued message', async () => {
23552348
const mockPopAllMessages = vi.fn();
2349+
mockPopAllMessages.mockReturnValue('Single message');
23562350
props.popAllMessages = mockPopAllMessages;
23572351
props.buffer.text = '';
23582352

@@ -2365,11 +2359,6 @@ describe('InputPrompt', () => {
23652359
});
23662360
await waitFor(() => expect(mockPopAllMessages).toHaveBeenCalled());
23672361

2368-
const callback = mockPopAllMessages.mock.calls[0][0];
2369-
await act(async () => {
2370-
callback('Single message');
2371-
});
2372-
23732362
expect(props.buffer.setText).toHaveBeenCalledWith('Single message');
23742363
unmount();
23752364
});
@@ -2409,6 +2398,7 @@ describe('InputPrompt', () => {
24092398

24102399
it('should navigate input history on fresh start when no queued messages exist', async () => {
24112400
const mockPopAllMessages = vi.fn();
2401+
mockPopAllMessages.mockReturnValue(undefined);
24122402
props.popAllMessages = mockPopAllMessages;
24132403
props.buffer.text = '';
24142404

@@ -2421,11 +2411,6 @@ describe('InputPrompt', () => {
24212411
});
24222412
await waitFor(() => expect(mockPopAllMessages).toHaveBeenCalled());
24232413

2424-
const callback = mockPopAllMessages.mock.calls[0][0];
2425-
await act(async () => {
2426-
callback(undefined);
2427-
});
2428-
24292414
expect(mockInputHistory.navigateUp).toHaveBeenCalled();
24302415
expect(props.buffer.setText).not.toHaveBeenCalled();
24312416

packages/cli/src/ui/components/InputPrompt.tsx

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ export interface InputPromptProps {
8383
isEmbeddedShellFocused?: boolean;
8484
setQueueErrorMessage: (message: string | null) => void;
8585
streamingState: StreamingState;
86-
popAllMessages?: (onPop: (messages: string | undefined) => void) => void;
86+
popAllMessages?: () => string | undefined;
8787
suggestionsPosition?: 'above' | 'below';
8888
setBannerVisible: (visible: boolean) => void;
8989
}
@@ -143,15 +143,6 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
143143
const pasteTimeoutRef = useRef<NodeJS.Timeout | null>(null);
144144
const innerBoxRef = useRef<DOMElement>(null);
145145

146-
const [dirs, setDirs] = useState<readonly string[]>(
147-
config.getWorkspaceContext().getDirectories(),
148-
);
149-
const dirsChanged = config.getWorkspaceContext().getDirectories();
150-
useEffect(() => {
151-
if (dirs.length !== dirsChanged.length) {
152-
setDirs(dirsChanged);
153-
}
154-
}, [dirs.length, dirsChanged]);
155146
const [reverseSearchActive, setReverseSearchActive] = useState(false);
156147
const [commandSearchActive, setCommandSearchActive] = useState(false);
157148
const [textBeforeReverseSearch, setTextBeforeReverseSearch] = useState('');
@@ -165,7 +156,6 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
165156

166157
const completion = useCommandCompletion(
167158
buffer,
168-
dirs,
169159
config.getTargetDir(),
170160
slashCommands,
171161
commandContext,
@@ -310,14 +300,13 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
310300
// Returns true if we should continue with input history navigation
311301
const tryLoadQueuedMessages = useCallback(() => {
312302
if (buffer.text.trim() === '' && popAllMessages) {
313-
popAllMessages((allMessages) => {
314-
if (allMessages) {
315-
buffer.setText(allMessages);
316-
} else {
317-
// No queued messages, proceed with input history
318-
inputHistory.navigateUp();
319-
}
320-
});
303+
const allMessages = popAllMessages();
304+
if (allMessages) {
305+
buffer.setText(allMessages);
306+
} else {
307+
// No queued messages, proceed with input history
308+
inputHistory.navigateUp();
309+
}
321310
return true; // We handled the up arrow key
322311
}
323312
return false;

packages/cli/src/ui/contexts/UIActionsContext.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export interface UIActions {
5151
handleResumeSession: (session: SessionInfo) => Promise<void>;
5252
handleDeleteSession: (session: SessionInfo) => Promise<void>;
5353
setQueueErrorMessage: (message: string | null) => void;
54-
popAllMessages: (onPop: (messages: string | undefined) => void) => void;
54+
popAllMessages: () => string | undefined;
5555
handleApiKeySubmit: (apiKey: string) => Promise<void>;
5656
handleApiKeyCancel: () => void;
5757
setBannerVisible: (visible: boolean) => void;

packages/cli/src/ui/hooks/useCommandCompletion.test.tsx

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,6 @@ describe('useCommandCompletion', () => {
9494
getEnablePromptCompletion: () => false,
9595
getGeminiClient: vi.fn(),
9696
} as unknown as Config;
97-
const testDirs: string[] = [];
9897
const testRootDir = '/';
9998

10099
// Helper to create real TextBuffer objects within renderHook
@@ -121,7 +120,6 @@ describe('useCommandCompletion', () => {
121120
const textBuffer = useTextBufferForTest(initialText, cursorOffset);
122121
const completion = useCommandCompletion(
123122
textBuffer,
124-
testDirs,
125123
testRootDir,
126124
[],
127125
mockCommandContext,
@@ -505,7 +503,6 @@ describe('useCommandCompletion', () => {
505503
const textBuffer = useTextBufferForTest('// This is a line comment');
506504
const completion = useCommandCompletion(
507505
textBuffer,
508-
testDirs,
509506
testRootDir,
510507
[],
511508
mockCommandContext,
@@ -538,7 +535,6 @@ describe('useCommandCompletion', () => {
538535
);
539536
const completion = useCommandCompletion(
540537
textBuffer,
541-
testDirs,
542538
testRootDir,
543539
[],
544540
mockCommandContext,
@@ -571,7 +567,6 @@ describe('useCommandCompletion', () => {
571567
);
572568
const completion = useCommandCompletion(
573569
textBuffer,
574-
testDirs,
575570
testRootDir,
576571
[],
577572
mockCommandContext,

packages/cli/src/ui/hooks/useCommandCompletion.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ export interface UseCommandCompletionReturn {
5757

5858
export function useCommandCompletion(
5959
buffer: TextBuffer,
60-
dirs: readonly string[],
6160
cwd: string,
6261
slashCommands: readonly SlashCommand[],
6362
commandContext: CommandContext,

packages/cli/src/ui/hooks/useMessageQueue.test.tsx

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -253,9 +253,7 @@ describe('useMessageQueue', () => {
253253
// Pop all messages
254254
let poppedMessages: string | undefined;
255255
act(() => {
256-
result.current.popAllMessages((messages) => {
257-
poppedMessages = messages;
258-
});
256+
poppedMessages = result.current.popAllMessages();
259257
});
260258

261259
expect(poppedMessages).toBe('Message 1\n\nMessage 2\n\nMessage 3');
@@ -271,9 +269,7 @@ describe('useMessageQueue', () => {
271269

272270
let poppedMessages: string | undefined = 'not-undefined';
273271
act(() => {
274-
result.current.popAllMessages((messages) => {
275-
poppedMessages = messages;
276-
});
272+
poppedMessages = result.current.popAllMessages();
277273
});
278274

279275
expect(poppedMessages).toBeUndefined();
@@ -293,9 +289,7 @@ describe('useMessageQueue', () => {
293289

294290
let poppedMessages: string | undefined;
295291
act(() => {
296-
result.current.popAllMessages((messages) => {
297-
poppedMessages = messages;
298-
});
292+
poppedMessages = result.current.popAllMessages();
299293
});
300294

301295
expect(poppedMessages).toBe('Single message');
@@ -315,7 +309,7 @@ describe('useMessageQueue', () => {
315309
});
316310

317311
act(() => {
318-
result.current.popAllMessages(() => {});
312+
result.current.popAllMessages();
319313
});
320314

321315
// Queue should be empty
@@ -325,9 +319,7 @@ describe('useMessageQueue', () => {
325319
// Popping again should return undefined
326320
let secondPop: string | undefined = 'not-undefined';
327321
act(() => {
328-
result.current.popAllMessages((messages) => {
329-
secondPop = messages;
330-
});
322+
secondPop = result.current.popAllMessages();
331323
});
332324

333325
expect(secondPop).toBeUndefined();
@@ -349,9 +341,7 @@ describe('useMessageQueue', () => {
349341
// Pop all messages
350342
let firstPop: string | undefined;
351343
act(() => {
352-
result.current.popAllMessages((messages) => {
353-
firstPop = messages;
354-
});
344+
firstPop = result.current.popAllMessages();
355345
});
356346

357347
expect(firstPop).toBe('First\n\nSecond');
@@ -365,9 +355,7 @@ describe('useMessageQueue', () => {
365355
// Pop again
366356
let secondPop: string | undefined;
367357
act(() => {
368-
result.current.popAllMessages((messages) => {
369-
secondPop = messages;
370-
});
358+
secondPop = result.current.popAllMessages();
371359
});
372360

373361
expect(secondPop).toBe('Third\n\nFourth');

packages/cli/src/ui/hooks/useMessageQueue.ts

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ export interface UseMessageQueueReturn {
1818
addMessage: (message: string) => void;
1919
clearQueue: () => void;
2020
getQueuedMessagesText: () => string;
21-
popAllMessages: (onPop: (messages: string | undefined) => void) => void;
21+
popAllMessages: () => string | undefined;
2222
}
2323

2424
/**
@@ -53,21 +53,14 @@ export function useMessageQueue({
5353
}, [messageQueue]);
5454

5555
// Pop all messages from the queue and return them as a single string
56-
const popAllMessages = useCallback(
57-
(onPop: (messages: string | undefined) => void) => {
58-
setMessageQueue((prev) => {
59-
if (prev.length === 0) {
60-
onPop(undefined);
61-
return prev;
62-
}
63-
// Join all messages with double newlines, same as when they're sent
64-
const allMessages = prev.join('\n\n');
65-
onPop(allMessages);
66-
return []; // Clear the entire queue
67-
});
68-
},
69-
[],
70-
);
56+
const popAllMessages = useCallback(() => {
57+
if (messageQueue.length === 0) {
58+
return undefined;
59+
}
60+
const allMessages = messageQueue.join('\n\n');
61+
setMessageQueue([]);
62+
return allMessages;
63+
}, [messageQueue]);
7164

7265
// Process queued messages when streaming becomes idle
7366
useEffect(() => {

0 commit comments

Comments
 (0)