Skip to content

Commit f96bc0f

Browse files
authored
Merge branch 'main' into tomm_symlink_write_fix
2 parents b18aaa1 + 6104717 commit f96bc0f

39 files changed

+868
-548
lines changed

GEMINI.md

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -97,24 +97,45 @@ TypeScript's power lies in its ability to provide static type checking, catching
9797

9898
- **Preferring `unknown` over `any`**: When you absolutely cannot determine the type of a value at compile time, and you're tempted to reach for any, consider using unknown instead. unknown is a type-safe counterpart to any. While a variable of type unknown can hold any value, you must perform type narrowing (e.g., using typeof or instanceof checks, or a type assertion) before you can perform any operations on it. This forces you to handle the unknown type explicitly, preventing accidental runtime errors.
9999

100-
```
100+
```ts
101101
function processValue(value: unknown) {
102-
if (typeof value === 'string') {
103-
// value is now safely a string
104-
console.log(value.toUpperCase());
105-
} else if (typeof value === 'number') {
106-
// value is now safely a number
107-
console.log(value * 2);
108-
}
109-
// Without narrowing, you cannot access properties or methods on 'value'
110-
// console.log(value.someProperty); // Error: Object is of type 'unknown'.
102+
if (typeof value === 'string') {
103+
// value is now safely a string
104+
console.log(value.toUpperCase());
105+
} else if (typeof value === 'number') {
106+
// value is now safely a number
107+
console.log(value * 2);
108+
}
109+
// Without narrowing, you cannot access properties or methods on 'value'
110+
// console.log(value.someProperty); // Error: Object is of type 'unknown'.
111111
}
112112
```
113113

114114
- **Type Assertions (`as Type`) - Use with Caution**: Type assertions tell the TypeScript compiler, "Trust me, I know what I'm doing; this is definitely of this type." While there are legitimate use cases (e.g., when dealing with external libraries that don't have perfect type definitions, or when you have more information than the compiler), they should be used sparingly and with extreme caution.
115115
- **Bypassing Type Checking**: Like `any`, type assertions bypass TypeScript's safety checks. If your assertion is incorrect, you introduce a runtime error that TypeScript would not have warned you about.
116116
- **Code Smell in Testing**: A common scenario where `any` or type assertions might be tempting is when trying to test "private" implementation details (e.g., spying on or stubbing an unexported function within a module). This is a strong indication of a "code smell" in your testing strategy and potentially your code structure. Instead of trying to force access to private internals, consider whether those internal details should be refactored into a separate module with a well-defined public API. This makes them inherently testable without compromising encapsulation.
117117

118+
### Type narrowing `switch` clauses
119+
120+
When authoring a switch clause over an enumeration or fixed list of items,
121+
always prefer to use the `checkExhaustive` helper method within the default
122+
clause of the switch. This will ensure that all of the possible options within
123+
the value or enumeration are used.
124+
125+
This helper method can be found in `packages/cli/src/utils/checks.ts`
126+
127+
Here's an example of using the helper method properly:
128+
129+
```
130+
switch (someValue) {
131+
case 1:
132+
case 2:
133+
// ...
134+
default:
135+
return checkExhaustive(someValue);
136+
}
137+
```
138+
118139
### Embracing JavaScript's Array Operators
119140

120141
To further enhance code cleanliness and promote safe functional programming practices, leverage JavaScript's rich set of array operators as much as possible. Methods like `.map()`, `.filter()`, `.reduce()`, `.slice()`, `.sort()`, and others are incredibly powerful for transforming and manipulating data collections in an immutable and declarative way.

packages/cli/src/ui/commands/directoryCommand.tsx

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -138,13 +138,11 @@ export const directoryCommand: SlashCommand = {
138138

139139
if (errors.length > 0) {
140140
addItem(
141-
{
142-
type: MessageType.ERROR,
143-
text: errors.join('\n'),
144-
},
141+
{ type: MessageType.ERROR, text: errors.join('\n') },
145142
Date.now(),
146143
);
147144
}
145+
return;
148146
},
149147
},
150148
{

packages/cli/src/ui/commands/mcpCommand.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -881,9 +881,14 @@ describe('mcpCommand', () => {
881881
}),
882882
getToolRegistry: vi.fn().mockResolvedValue(mockToolRegistry),
883883
getGeminiClient: vi.fn().mockReturnValue(mockGeminiClient),
884+
getPromptRegistry: vi.fn().mockResolvedValue({
885+
removePromptsByServer: vi.fn(),
886+
}),
884887
},
885888
},
886889
});
890+
// Mock the reloadCommands function
891+
context.ui.reloadCommands = vi.fn();
887892

888893
const { MCPOAuthProvider } = await import('@google/gemini-cli-core');
889894

@@ -901,6 +906,7 @@ describe('mcpCommand', () => {
901906
'test-server',
902907
);
903908
expect(mockGeminiClient.setTools).toHaveBeenCalled();
909+
expect(context.ui.reloadCommands).toHaveBeenCalledTimes(1);
904910

905911
expect(isMessageAction(result)).toBe(true);
906912
if (isMessageAction(result)) {
@@ -985,6 +991,8 @@ describe('mcpCommand', () => {
985991
},
986992
},
987993
});
994+
// Mock the reloadCommands function, which is new logic.
995+
context.ui.reloadCommands = vi.fn();
988996

989997
const refreshCommand = mcpCommand.subCommands?.find(
990998
(cmd) => cmd.name === 'refresh',
@@ -1002,6 +1010,7 @@ describe('mcpCommand', () => {
10021010
);
10031011
expect(mockToolRegistry.discoverMcpTools).toHaveBeenCalled();
10041012
expect(mockGeminiClient.setTools).toHaveBeenCalled();
1013+
expect(context.ui.reloadCommands).toHaveBeenCalledTimes(1);
10051014

10061015
expect(isMessageAction(result)).toBe(true);
10071016
if (isMessageAction(result)) {

packages/cli/src/ui/commands/mcpCommand.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,9 @@ const authCommand: SlashCommand = {
417417
await geminiClient.setTools();
418418
}
419419

420+
// Reload the slash commands to reflect the changes.
421+
context.ui.reloadCommands();
422+
420423
return {
421424
type: 'message',
422425
messageType: 'info',
@@ -507,6 +510,9 @@ const refreshCommand: SlashCommand = {
507510
await geminiClient.setTools();
508511
}
509512

513+
// Reload the slash commands to reflect the changes.
514+
context.ui.reloadCommands();
515+
510516
return getMcpStatus(context, false, false, false);
511517
},
512518
};

packages/cli/src/ui/commands/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ export interface CommandContext {
6161
toggleCorgiMode: () => void;
6262
toggleVimEnabled: () => Promise<boolean>;
6363
setGeminiMdFileCount: (count: number) => void;
64+
reloadCommands: () => void;
6465
};
6566
// Session-specific data
6667
session: {

packages/cli/src/ui/components/shared/vim-buffer-actions.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
findWordEndInLine,
2020
} from './text-buffer.js';
2121
import { cpLen, toCodePoints } from '../../utils/textUtils.js';
22+
import { assumeExhaustive } from '../../../utils/checks.js';
2223

2324
// Check if we're at the end of a base word (on the last base character)
2425
// Returns true if current position has a base character followed only by combining marks until non-word
@@ -806,7 +807,7 @@ export function handleVimAction(
806807

807808
default: {
808809
// This should never happen if TypeScript is working correctly
809-
const _exhaustiveCheck: never = action;
810+
assumeExhaustive(action);
810811
return state;
811812
}
812813
}

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ export const useSlashCommandProcessor = (
5757
) => {
5858
const session = useSessionStats();
5959
const [commands, setCommands] = useState<readonly SlashCommand[]>([]);
60+
const [reloadTrigger, setReloadTrigger] = useState(0);
61+
62+
const reloadCommands = useCallback(() => {
63+
setReloadTrigger((v) => v + 1);
64+
}, []);
6065
const [shellConfirmationRequest, setShellConfirmationRequest] =
6166
useState<null | {
6267
commands: string[];
@@ -172,6 +177,7 @@ export const useSlashCommandProcessor = (
172177
toggleCorgiMode,
173178
toggleVimEnabled,
174179
setGeminiMdFileCount,
180+
reloadCommands,
175181
},
176182
session: {
177183
stats: session.stats,
@@ -195,6 +201,7 @@ export const useSlashCommandProcessor = (
195201
toggleVimEnabled,
196202
sessionShellAllowlist,
197203
setGeminiMdFileCount,
204+
reloadCommands,
198205
],
199206
);
200207

@@ -220,7 +227,7 @@ export const useSlashCommandProcessor = (
220227
return () => {
221228
controller.abort();
222229
};
223-
}, [config, ideMode]);
230+
}, [config, ideMode, reloadTrigger]);
224231

225232
const handleSlashCommand = useCallback(
226233
async (

packages/cli/src/utils/checks.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/**
2+
* @license
3+
* Copyright 2025 Google LLC
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
/* Fail to compile on unexpected values. */
8+
export function assumeExhaustive(_value: never): void {}
9+
10+
/**
11+
* Throws an exception on unexpected values.
12+
*
13+
* A common use case is switch statements:
14+
* switch(enumValue) {
15+
* case Enum.A:
16+
* case Enum.B:
17+
* break;
18+
* default:
19+
* checkExhaustive(enumValue);
20+
* }
21+
*/
22+
export function checkExhaustive(
23+
value: never,
24+
msg = `unexpected value ${value}!`,
25+
): never {
26+
assumeExhaustive(value);
27+
throw new Error(msg);
28+
}

packages/core/src/config/config.test.ts

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
} from '../telemetry/index.js';
1616
import {
1717
AuthType,
18+
ContentGeneratorConfig,
1819
createContentGeneratorConfig,
1920
} from '../core/contentGenerator.js';
2021
import { GeminiClient } from '../core/client.js';
@@ -249,6 +250,7 @@ describe('Server Config (config.ts)', () => {
249250
// Verify that history was restored to the new client
250251
expect(mockNewClient.setHistory).toHaveBeenCalledWith(
251252
mockExistingHistory,
253+
{ stripThoughts: false },
252254
);
253255
});
254256

@@ -282,6 +284,92 @@ describe('Server Config (config.ts)', () => {
282284
// Verify that setHistory was not called since there was no existing history
283285
expect(mockNewClient.setHistory).not.toHaveBeenCalled();
284286
});
287+
288+
it('should strip thoughts when switching from GenAI to Vertex', async () => {
289+
const config = new Config(baseParams);
290+
const mockContentConfig = {
291+
model: 'gemini-pro',
292+
apiKey: 'test-key',
293+
authType: AuthType.USE_GEMINI,
294+
};
295+
(
296+
config as unknown as { contentGeneratorConfig: ContentGeneratorConfig }
297+
).contentGeneratorConfig = mockContentConfig;
298+
299+
(createContentGeneratorConfig as Mock).mockReturnValue({
300+
...mockContentConfig,
301+
authType: AuthType.LOGIN_WITH_GOOGLE,
302+
});
303+
304+
const mockExistingHistory = [
305+
{ role: 'user', parts: [{ text: 'Hello' }] },
306+
];
307+
const mockExistingClient = {
308+
isInitialized: vi.fn().mockReturnValue(true),
309+
getHistory: vi.fn().mockReturnValue(mockExistingHistory),
310+
};
311+
const mockNewClient = {
312+
isInitialized: vi.fn().mockReturnValue(true),
313+
getHistory: vi.fn().mockReturnValue([]),
314+
setHistory: vi.fn(),
315+
initialize: vi.fn().mockResolvedValue(undefined),
316+
};
317+
318+
(
319+
config as unknown as { geminiClient: typeof mockExistingClient }
320+
).geminiClient = mockExistingClient;
321+
(GeminiClient as Mock).mockImplementation(() => mockNewClient);
322+
323+
await config.refreshAuth(AuthType.LOGIN_WITH_GOOGLE);
324+
325+
expect(mockNewClient.setHistory).toHaveBeenCalledWith(
326+
mockExistingHistory,
327+
{ stripThoughts: true },
328+
);
329+
});
330+
331+
it('should not strip thoughts when switching from Vertex to GenAI', async () => {
332+
const config = new Config(baseParams);
333+
const mockContentConfig = {
334+
model: 'gemini-pro',
335+
apiKey: 'test-key',
336+
authType: AuthType.LOGIN_WITH_GOOGLE,
337+
};
338+
(
339+
config as unknown as { contentGeneratorConfig: ContentGeneratorConfig }
340+
).contentGeneratorConfig = mockContentConfig;
341+
342+
(createContentGeneratorConfig as Mock).mockReturnValue({
343+
...mockContentConfig,
344+
authType: AuthType.USE_GEMINI,
345+
});
346+
347+
const mockExistingHistory = [
348+
{ role: 'user', parts: [{ text: 'Hello' }] },
349+
];
350+
const mockExistingClient = {
351+
isInitialized: vi.fn().mockReturnValue(true),
352+
getHistory: vi.fn().mockReturnValue(mockExistingHistory),
353+
};
354+
const mockNewClient = {
355+
isInitialized: vi.fn().mockReturnValue(true),
356+
getHistory: vi.fn().mockReturnValue([]),
357+
setHistory: vi.fn(),
358+
initialize: vi.fn().mockResolvedValue(undefined),
359+
};
360+
361+
(
362+
config as unknown as { geminiClient: typeof mockExistingClient }
363+
).geminiClient = mockExistingClient;
364+
(GeminiClient as Mock).mockImplementation(() => mockNewClient);
365+
366+
await config.refreshAuth(AuthType.USE_GEMINI);
367+
368+
expect(mockNewClient.setHistory).toHaveBeenCalledWith(
369+
mockExistingHistory,
370+
{ stripThoughts: false },
371+
);
372+
});
285373
});
286374

287375
it('Config constructor should store userMemory correctly', () => {

packages/core/src/config/config.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,13 +379,21 @@ export class Config {
379379
const newGeminiClient = new GeminiClient(this);
380380
await newGeminiClient.initialize(newContentGeneratorConfig);
381381

382+
// Vertex and Genai have incompatible encryption and sending history with
383+
// throughtSignature from Genai to Vertex will fail, we need to strip them
384+
const fromGenaiToVertex =
385+
this.contentGeneratorConfig?.authType === AuthType.USE_GEMINI &&
386+
authMethod === AuthType.LOGIN_WITH_GOOGLE;
387+
382388
// Only assign to instance properties after successful initialization
383389
this.contentGeneratorConfig = newContentGeneratorConfig;
384390
this.geminiClient = newGeminiClient;
385391

386392
// Restore the conversation history to the new client
387393
if (existingHistory.length > 0) {
388-
this.geminiClient.setHistory(existingHistory);
394+
this.geminiClient.setHistory(existingHistory, {
395+
stripThoughts: fromGenaiToVertex,
396+
});
389397
}
390398

391399
// Reset the session flag since we're explicitly changing auth and using default model

0 commit comments

Comments
 (0)