From 2ca183ffc9d2ab38b4835de1b657003963abe643 Mon Sep 17 00:00:00 2001 From: christine betts Date: Thu, 12 Feb 2026 11:29:06 -0500 Subject: [PATCH] Show notification when there's a conflict with an extensions command (#17890) --- packages/cli/src/gemini.tsx | 6 + .../cli/src/services/CommandService.test.ts | 113 ++++++++++++++ packages/cli/src/services/CommandService.ts | 59 ++++++- .../services/SlashCommandConflictHandler.ts | 54 +++++++ .../ui/hooks/slashCommandProcessor.test.tsx | 144 +++++++++++++++++- .../cli/src/ui/hooks/slashCommandProcessor.ts | 5 + packages/core/src/utils/events.ts | 18 +++ 7 files changed, 395 insertions(+), 4 deletions(-) create mode 100644 packages/cli/src/services/SlashCommandConflictHandler.ts diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index e138cfe03a..31e8bd433b 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -104,6 +104,7 @@ import { TerminalProvider } from './ui/contexts/TerminalContext.js'; import { setupTerminalAndTheme } from './utils/terminalTheme.js'; import { profiler } from './ui/components/DebugProfiler.js'; import { runDeferredCommand } from './deferred.js'; +import { SlashCommandConflictHandler } from './services/SlashCommandConflictHandler.js'; const SLOW_RENDER_MS = 200; @@ -335,6 +336,11 @@ export async function main() { }); setupUnhandledRejectionHandler(); + + const slashCommandConflictHandler = new SlashCommandConflictHandler(); + slashCommandConflictHandler.start(); + registerCleanup(() => slashCommandConflictHandler.stop()); + const loadSettingsHandle = startupProfiler.start('load_settings'); const settings = loadSettings(); loadSettingsHandle?.end(); diff --git a/packages/cli/src/services/CommandService.test.ts b/packages/cli/src/services/CommandService.test.ts index 31dfdcace8..ea906a3da6 100644 --- a/packages/cli/src/services/CommandService.test.ts +++ b/packages/cli/src/services/CommandService.test.ts @@ -350,4 +350,117 @@ describe('CommandService', () => { expect(deployExtension).toBeDefined(); expect(deployExtension?.description).toBe('[gcp] Deploy to Google Cloud'); }); + + it('should report conflicts via getConflicts', async () => { + const builtinCommand = createMockCommand('deploy', CommandKind.BUILT_IN); + const extensionCommand = { + ...createMockCommand('deploy', CommandKind.FILE), + extensionName: 'firebase', + }; + + const mockLoader = new MockCommandLoader([ + builtinCommand, + extensionCommand, + ]); + + const service = await CommandService.create( + [mockLoader], + new AbortController().signal, + ); + + const conflicts = service.getConflicts(); + expect(conflicts).toHaveLength(1); + + expect(conflicts[0]).toMatchObject({ + name: 'deploy', + winner: builtinCommand, + losers: [ + { + renamedTo: 'firebase.deploy', + command: expect.objectContaining({ + name: 'deploy', + extensionName: 'firebase', + }), + }, + ], + }); + }); + + it('should report extension vs extension conflicts correctly', async () => { + // Both extensions try to register 'deploy' + const extension1Command = { + ...createMockCommand('deploy', CommandKind.FILE), + extensionName: 'firebase', + }; + const extension2Command = { + ...createMockCommand('deploy', CommandKind.FILE), + extensionName: 'aws', + }; + + const mockLoader = new MockCommandLoader([ + extension1Command, + extension2Command, + ]); + + const service = await CommandService.create( + [mockLoader], + new AbortController().signal, + ); + + const conflicts = service.getConflicts(); + expect(conflicts).toHaveLength(1); + + expect(conflicts[0]).toMatchObject({ + name: 'deploy', + winner: expect.objectContaining({ + name: 'deploy', + extensionName: 'firebase', + }), + losers: [ + { + renamedTo: 'aws.deploy', // ext2 is 'aws' and it lost because it was second in the list + command: expect.objectContaining({ + name: 'deploy', + extensionName: 'aws', + }), + }, + ], + }); + }); + + it('should report multiple conflicts for the same command name', async () => { + const builtinCommand = createMockCommand('deploy', CommandKind.BUILT_IN); + const ext1 = { + ...createMockCommand('deploy', CommandKind.FILE), + extensionName: 'ext1', + }; + const ext2 = { + ...createMockCommand('deploy', CommandKind.FILE), + extensionName: 'ext2', + }; + + const mockLoader = new MockCommandLoader([builtinCommand, ext1, ext2]); + + const service = await CommandService.create( + [mockLoader], + new AbortController().signal, + ); + + const conflicts = service.getConflicts(); + expect(conflicts).toHaveLength(1); + expect(conflicts[0].name).toBe('deploy'); + expect(conflicts[0].losers).toHaveLength(2); + expect(conflicts[0].losers).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + renamedTo: 'ext1.deploy', + command: expect.objectContaining({ extensionName: 'ext1' }), + }), + expect.objectContaining({ + renamedTo: 'ext2.deploy', + command: expect.objectContaining({ extensionName: 'ext2' }), + }), + ]), + ); + }); }); diff --git a/packages/cli/src/services/CommandService.ts b/packages/cli/src/services/CommandService.ts index 0e29a81d00..bd42226a32 100644 --- a/packages/cli/src/services/CommandService.ts +++ b/packages/cli/src/services/CommandService.ts @@ -4,10 +4,19 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { debugLogger } from '@google/gemini-cli-core'; +import { debugLogger, coreEvents } from '@google/gemini-cli-core'; import type { SlashCommand } from '../ui/commands/types.js'; import type { ICommandLoader } from './types.js'; +export interface CommandConflict { + name: string; + winner: SlashCommand; + losers: Array<{ + command: SlashCommand; + renamedTo: string; + }>; +} + /** * Orchestrates the discovery and loading of all slash commands for the CLI. * @@ -23,8 +32,12 @@ export class CommandService { /** * Private constructor to enforce the use of the async factory. * @param commands A readonly array of the fully loaded and de-duplicated commands. + * @param conflicts A readonly array of conflicts that occurred during loading. */ - private constructor(private readonly commands: readonly SlashCommand[]) {} + private constructor( + private readonly commands: readonly SlashCommand[], + private readonly conflicts: readonly CommandConflict[], + ) {} /** * Asynchronously creates and initializes a new CommandService instance. @@ -63,11 +76,14 @@ export class CommandService { } const commandMap = new Map(); + const conflictsMap = new Map(); + for (const cmd of allCommands) { let finalName = cmd.name; // Extension commands get renamed if they conflict with existing commands if (cmd.extensionName && commandMap.has(cmd.name)) { + const winner = commandMap.get(cmd.name)!; let renamedName = `${cmd.extensionName}.${cmd.name}`; let suffix = 1; @@ -78,6 +94,19 @@ export class CommandService { } finalName = renamedName; + + if (!conflictsMap.has(cmd.name)) { + conflictsMap.set(cmd.name, { + name: cmd.name, + winner, + losers: [], + }); + } + + conflictsMap.get(cmd.name)!.losers.push({ + command: cmd, + renamedTo: finalName, + }); } commandMap.set(finalName, { @@ -86,8 +115,23 @@ export class CommandService { }); } + const conflicts = Array.from(conflictsMap.values()); + if (conflicts.length > 0) { + coreEvents.emitSlashCommandConflicts( + conflicts.flatMap((c) => + c.losers.map((l) => ({ + name: c.name, + renamedTo: l.renamedTo, + loserExtensionName: l.command.extensionName, + winnerExtensionName: c.winner.extensionName, + })), + ), + ); + } + const finalCommands = Object.freeze(Array.from(commandMap.values())); - return new CommandService(finalCommands); + const finalConflicts = Object.freeze(conflicts); + return new CommandService(finalCommands, finalConflicts); } /** @@ -101,4 +145,13 @@ export class CommandService { getCommands(): readonly SlashCommand[] { return this.commands; } + + /** + * Retrieves the list of conflicts that occurred during command loading. + * + * @returns A readonly array of command conflicts. + */ + getConflicts(): readonly CommandConflict[] { + return this.conflicts; + } } diff --git a/packages/cli/src/services/SlashCommandConflictHandler.ts b/packages/cli/src/services/SlashCommandConflictHandler.ts new file mode 100644 index 0000000000..31e110732b --- /dev/null +++ b/packages/cli/src/services/SlashCommandConflictHandler.ts @@ -0,0 +1,54 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + coreEvents, + CoreEvent, + type SlashCommandConflictsPayload, +} from '@google/gemini-cli-core'; + +export class SlashCommandConflictHandler { + private notifiedConflicts = new Set(); + + constructor() { + this.handleConflicts = this.handleConflicts.bind(this); + } + + start() { + coreEvents.on(CoreEvent.SlashCommandConflicts, this.handleConflicts); + } + + stop() { + coreEvents.off(CoreEvent.SlashCommandConflicts, this.handleConflicts); + } + + private handleConflicts(payload: SlashCommandConflictsPayload) { + const newConflicts = payload.conflicts.filter((c) => { + const key = `${c.name}:${c.loserExtensionName}`; + if (this.notifiedConflicts.has(key)) { + return false; + } + this.notifiedConflicts.add(key); + return true; + }); + + if (newConflicts.length > 0) { + const conflictMessages = newConflicts + .map((c) => { + const winnerSource = c.winnerExtensionName + ? `extension '${c.winnerExtensionName}'` + : 'an existing command'; + return `- Command '/${c.name}' from extension '${c.loserExtensionName}' was renamed to '/${c.renamedTo}' because it conflicts with ${winnerSource}.`; + }) + .join('\n'); + + coreEvents.emitFeedback( + 'info', + `Command conflicts detected:\n${conflictMessages}`, + ); + } + } +} diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx b/packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx index 049720d58a..11f47e12d3 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx @@ -18,10 +18,13 @@ import { FileCommandLoader } from '../../services/FileCommandLoader.js'; import { McpPromptLoader } from '../../services/McpPromptLoader.js'; import { type GeminiClient, + type UserFeedbackPayload, SlashCommandStatus, makeFakeConfig, coreEvents, + CoreEvent, } from '@google/gemini-cli-core'; +import { SlashCommandConflictHandler } from '../../services/SlashCommandConflictHandler.js'; const { logSlashCommand, @@ -182,6 +185,26 @@ describe('useSlashCommandProcessor', () => { mockFileLoadCommands.mockResolvedValue(Object.freeze(fileCommands)); mockMcpLoadCommands.mockResolvedValue(Object.freeze(mcpCommands)); + const conflictHandler = new SlashCommandConflictHandler(); + conflictHandler.start(); + + const handleFeedback = (payload: UserFeedbackPayload) => { + let type = MessageType.INFO; + if (payload.severity === 'error') { + type = MessageType.ERROR; + } else if (payload.severity === 'warning') { + type = MessageType.WARNING; + } + mockAddItem( + { + type, + text: payload.message, + }, + Date.now(), + ); + }; + coreEvents.on(CoreEvent.UserFeedback, handleFeedback); + let result!: { current: ReturnType }; let unmount!: () => void; let rerender!: (props?: unknown) => void; @@ -228,7 +251,11 @@ describe('useSlashCommandProcessor', () => { rerender = hook.rerender; }); - unmountHook = async () => unmount(); + unmountHook = async () => { + conflictHandler.stop(); + coreEvents.off(CoreEvent.UserFeedback, handleFeedback); + unmount(); + }; await waitFor(() => { expect(result.current.slashCommands).toBeDefined(); @@ -1052,4 +1079,119 @@ describe('useSlashCommandProcessor', () => { expect(result.current.slashCommands).toEqual([newCommand]), ); }); + + describe('Conflict Notifications', () => { + it('should display a warning when a command conflict occurs', async () => { + const builtinCommand = createTestCommand({ name: 'deploy' }); + const extensionCommand = createTestCommand( + { + name: 'deploy', + extensionName: 'firebase', + }, + CommandKind.FILE, + ); + + const result = await setupProcessorHook({ + builtinCommands: [builtinCommand], + fileCommands: [extensionCommand], + }); + + await waitFor(() => expect(result.current.slashCommands).toHaveLength(2)); + + expect(mockAddItem).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageType.INFO, + text: expect.stringContaining('Command conflicts detected'), + }), + expect.any(Number), + ); + + expect(mockAddItem).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageType.INFO, + text: expect.stringContaining( + "- Command '/deploy' from extension 'firebase' was renamed", + ), + }), + expect.any(Number), + ); + }); + + it('should deduplicate conflict warnings across re-renders', async () => { + const builtinCommand = createTestCommand({ name: 'deploy' }); + const extensionCommand = createTestCommand( + { + name: 'deploy', + extensionName: 'firebase', + }, + CommandKind.FILE, + ); + + const result = await setupProcessorHook({ + builtinCommands: [builtinCommand], + fileCommands: [extensionCommand], + }); + + await waitFor(() => expect(result.current.slashCommands).toHaveLength(2)); + + // First notification + expect(mockAddItem).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageType.INFO, + text: expect.stringContaining('Command conflicts detected'), + }), + expect.any(Number), + ); + + mockAddItem.mockClear(); + + // Trigger a reload or re-render + await act(async () => { + result.current.commandContext.ui.reloadCommands(); + }); + + // Wait a bit for effect to run + await new Promise((resolve) => setTimeout(resolve, 100)); + + // Should NOT have notified again + expect(mockAddItem).not.toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageType.INFO, + text: expect.stringContaining('Command conflicts detected'), + }), + expect.any(Number), + ); + }); + + it('should correctly identify the winner extension in the message', async () => { + const ext1Command = createTestCommand( + { + name: 'deploy', + extensionName: 'firebase', + }, + CommandKind.FILE, + ); + const ext2Command = createTestCommand( + { + name: 'deploy', + extensionName: 'aws', + }, + CommandKind.FILE, + ); + + const result = await setupProcessorHook({ + fileCommands: [ext1Command, ext2Command], + }); + + await waitFor(() => expect(result.current.slashCommands).toHaveLength(2)); + + expect(mockAddItem).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageType.INFO, + text: expect.stringContaining("conflicts with extension 'firebase'"), + }), + expect.any(Number), + ); + }); + }); }); diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.ts index 7289906a36..2c6c463e42 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.ts @@ -329,6 +329,11 @@ export const useSlashCommandProcessor = ( ], controller.signal, ); + + if (controller.signal.aborted) { + return; + } + setCommands(commandService.getCommands()); })(); diff --git a/packages/core/src/utils/events.ts b/packages/core/src/utils/events.ts index 5bd3c0f206..8784da07a2 100644 --- a/packages/core/src/utils/events.ts +++ b/packages/core/src/utils/events.ts @@ -127,6 +127,17 @@ export interface AgentsDiscoveredPayload { agents: AgentDefinition[]; } +export interface SlashCommandConflict { + name: string; + renamedTo: string; + loserExtensionName?: string; + winnerExtensionName?: string; +} + +export interface SlashCommandConflictsPayload { + conflicts: SlashCommandConflict[]; +} + /** * Payload for the 'quota-changed' event. */ @@ -155,6 +166,7 @@ export enum CoreEvent { AgentsDiscovered = 'agents-discovered', RequestEditorSelection = 'request-editor-selection', EditorSelected = 'editor-selected', + SlashCommandConflicts = 'slash-command-conflicts', QuotaChanged = 'quota-changed', } @@ -185,6 +197,7 @@ export interface CoreEvents extends ExtensionEvents { [CoreEvent.AgentsDiscovered]: [AgentsDiscoveredPayload]; [CoreEvent.RequestEditorSelection]: never[]; [CoreEvent.EditorSelected]: [EditorSelectedPayload]; + [CoreEvent.SlashCommandConflicts]: [SlashCommandConflictsPayload]; } type EventBacklogItem = { @@ -322,6 +335,11 @@ export class CoreEventEmitter extends EventEmitter { this._emitOrQueue(CoreEvent.AgentsDiscovered, payload); } + emitSlashCommandConflicts(conflicts: SlashCommandConflict[]): void { + const payload: SlashCommandConflictsPayload = { conflicts }; + this._emitOrQueue(CoreEvent.SlashCommandConflicts, payload); + } + /** * Notifies subscribers that the quota has changed. */