Show notification when there's a conflict with an extensions command (#17890)

This commit is contained in:
christine betts
2026-02-12 11:29:06 -05:00
committed by GitHub
parent 099aa9621c
commit 2ca183ffc9
7 changed files with 395 additions and 4 deletions

View File

@@ -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();

View File

@@ -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' }),
}),
]),
);
});
});

View File

@@ -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<string, SlashCommand>();
const conflictsMap = new Map<string, CommandConflict>();
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;
}
}

View File

@@ -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<string>();
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}`,
);
}
}
}

View File

@@ -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<typeof useSlashCommandProcessor> };
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),
);
});
});
});

View File

@@ -329,6 +329,11 @@ export const useSlashCommandProcessor = (
],
controller.signal,
);
if (controller.signal.aborted) {
return;
}
setCommands(commandService.getCommands());
})();

View File

@@ -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<CoreEvents> {
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.
*/