mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-28 05:55:17 -07:00
feat(cli): implement dot-prefixing for slash command conflicts (#20979)
This commit is contained in:
@@ -17,21 +17,9 @@ const createMockCommand = (name: string, kind: CommandKind): SlashCommand => ({
|
||||
action: vi.fn(),
|
||||
});
|
||||
|
||||
const mockCommandA = createMockCommand('command-a', CommandKind.BUILT_IN);
|
||||
const mockCommandB = createMockCommand('command-b', CommandKind.BUILT_IN);
|
||||
const mockCommandC = createMockCommand('command-c', CommandKind.FILE);
|
||||
const mockCommandB_Override = createMockCommand('command-b', CommandKind.FILE);
|
||||
|
||||
class MockCommandLoader implements ICommandLoader {
|
||||
private commandsToLoad: SlashCommand[];
|
||||
|
||||
constructor(commandsToLoad: SlashCommand[]) {
|
||||
this.commandsToLoad = commandsToLoad;
|
||||
}
|
||||
|
||||
loadCommands = vi.fn(
|
||||
async (): Promise<SlashCommand[]> => Promise.resolve(this.commandsToLoad),
|
||||
);
|
||||
constructor(private readonly commands: SlashCommand[]) {}
|
||||
loadCommands = vi.fn(async () => Promise.resolve(this.commands));
|
||||
}
|
||||
|
||||
describe('CommandService', () => {
|
||||
@@ -43,424 +31,74 @@ describe('CommandService', () => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('should load commands from a single loader', async () => {
|
||||
const mockLoader = new MockCommandLoader([mockCommandA, mockCommandB]);
|
||||
const service = await CommandService.create(
|
||||
[mockLoader],
|
||||
new AbortController().signal,
|
||||
);
|
||||
describe('basic loading', () => {
|
||||
it('should aggregate commands from multiple successful loaders', async () => {
|
||||
const cmdA = createMockCommand('a', CommandKind.BUILT_IN);
|
||||
const cmdB = createMockCommand('b', CommandKind.USER_FILE);
|
||||
const service = await CommandService.create(
|
||||
[new MockCommandLoader([cmdA]), new MockCommandLoader([cmdB])],
|
||||
new AbortController().signal,
|
||||
);
|
||||
|
||||
const commands = service.getCommands();
|
||||
expect(service.getCommands()).toHaveLength(2);
|
||||
expect(service.getCommands()).toEqual(
|
||||
expect.arrayContaining([cmdA, cmdB]),
|
||||
);
|
||||
});
|
||||
|
||||
expect(mockLoader.loadCommands).toHaveBeenCalledTimes(1);
|
||||
expect(commands).toHaveLength(2);
|
||||
expect(commands).toEqual(
|
||||
expect.arrayContaining([mockCommandA, mockCommandB]),
|
||||
);
|
||||
});
|
||||
it('should handle empty loaders and failed loaders gracefully', async () => {
|
||||
const cmdA = createMockCommand('a', CommandKind.BUILT_IN);
|
||||
const failingLoader = new MockCommandLoader([]);
|
||||
vi.spyOn(failingLoader, 'loadCommands').mockRejectedValue(
|
||||
new Error('fail'),
|
||||
);
|
||||
|
||||
it('should aggregate commands from multiple loaders', async () => {
|
||||
const loader1 = new MockCommandLoader([mockCommandA]);
|
||||
const loader2 = new MockCommandLoader([mockCommandC]);
|
||||
const service = await CommandService.create(
|
||||
[loader1, loader2],
|
||||
new AbortController().signal,
|
||||
);
|
||||
const service = await CommandService.create(
|
||||
[
|
||||
new MockCommandLoader([cmdA]),
|
||||
new MockCommandLoader([]),
|
||||
failingLoader,
|
||||
],
|
||||
new AbortController().signal,
|
||||
);
|
||||
|
||||
const commands = service.getCommands();
|
||||
expect(service.getCommands()).toHaveLength(1);
|
||||
expect(service.getCommands()[0].name).toBe('a');
|
||||
expect(debugLogger.debug).toHaveBeenCalledWith(
|
||||
'A command loader failed:',
|
||||
expect.any(Error),
|
||||
);
|
||||
});
|
||||
|
||||
expect(loader1.loadCommands).toHaveBeenCalledTimes(1);
|
||||
expect(loader2.loadCommands).toHaveBeenCalledTimes(1);
|
||||
expect(commands).toHaveLength(2);
|
||||
expect(commands).toEqual(
|
||||
expect.arrayContaining([mockCommandA, mockCommandC]),
|
||||
);
|
||||
});
|
||||
it('should return a readonly array of commands', async () => {
|
||||
const service = await CommandService.create(
|
||||
[new MockCommandLoader([createMockCommand('a', CommandKind.BUILT_IN)])],
|
||||
new AbortController().signal,
|
||||
);
|
||||
expect(() => (service.getCommands() as unknown[]).push({})).toThrow();
|
||||
});
|
||||
|
||||
it('should override commands from earlier loaders with those from later loaders', async () => {
|
||||
const loader1 = new MockCommandLoader([mockCommandA, mockCommandB]);
|
||||
const loader2 = new MockCommandLoader([
|
||||
mockCommandB_Override,
|
||||
mockCommandC,
|
||||
]);
|
||||
const service = await CommandService.create(
|
||||
[loader1, loader2],
|
||||
new AbortController().signal,
|
||||
);
|
||||
|
||||
const commands = service.getCommands();
|
||||
|
||||
expect(commands).toHaveLength(3); // Should be A, C, and the overridden B.
|
||||
|
||||
// The final list should contain the override from the *last* loader.
|
||||
const commandB = commands.find((cmd) => cmd.name === 'command-b');
|
||||
expect(commandB).toBeDefined();
|
||||
expect(commandB?.kind).toBe(CommandKind.FILE); // Verify it's the overridden version.
|
||||
expect(commandB).toEqual(mockCommandB_Override);
|
||||
|
||||
// Ensure the other commands are still present.
|
||||
expect(commands).toEqual(
|
||||
expect.arrayContaining([
|
||||
mockCommandA,
|
||||
mockCommandC,
|
||||
mockCommandB_Override,
|
||||
]),
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle loaders that return an empty array of commands gracefully', async () => {
|
||||
const loader1 = new MockCommandLoader([mockCommandA]);
|
||||
const emptyLoader = new MockCommandLoader([]);
|
||||
const loader3 = new MockCommandLoader([mockCommandB]);
|
||||
const service = await CommandService.create(
|
||||
[loader1, emptyLoader, loader3],
|
||||
new AbortController().signal,
|
||||
);
|
||||
|
||||
const commands = service.getCommands();
|
||||
|
||||
expect(emptyLoader.loadCommands).toHaveBeenCalledTimes(1);
|
||||
expect(commands).toHaveLength(2);
|
||||
expect(commands).toEqual(
|
||||
expect.arrayContaining([mockCommandA, mockCommandB]),
|
||||
);
|
||||
});
|
||||
|
||||
it('should load commands from successful loaders even if one fails', async () => {
|
||||
const successfulLoader = new MockCommandLoader([mockCommandA]);
|
||||
const failingLoader = new MockCommandLoader([]);
|
||||
const error = new Error('Loader failed');
|
||||
vi.spyOn(failingLoader, 'loadCommands').mockRejectedValue(error);
|
||||
|
||||
const service = await CommandService.create(
|
||||
[successfulLoader, failingLoader],
|
||||
new AbortController().signal,
|
||||
);
|
||||
|
||||
const commands = service.getCommands();
|
||||
expect(commands).toHaveLength(1);
|
||||
expect(commands).toEqual([mockCommandA]);
|
||||
expect(debugLogger.debug).toHaveBeenCalledWith(
|
||||
'A command loader failed:',
|
||||
error,
|
||||
);
|
||||
});
|
||||
|
||||
it('getCommands should return a readonly array that cannot be mutated', async () => {
|
||||
const service = await CommandService.create(
|
||||
[new MockCommandLoader([mockCommandA])],
|
||||
new AbortController().signal,
|
||||
);
|
||||
|
||||
const commands = service.getCommands();
|
||||
|
||||
// Expect it to throw a TypeError at runtime because the array is frozen.
|
||||
expect(() => {
|
||||
// @ts-expect-error - Testing immutability is intentional here.
|
||||
commands.push(mockCommandB);
|
||||
}).toThrow();
|
||||
|
||||
// Verify the original array was not mutated.
|
||||
expect(service.getCommands()).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('should pass the abort signal to all loaders', async () => {
|
||||
const controller = new AbortController();
|
||||
const signal = controller.signal;
|
||||
|
||||
const loader1 = new MockCommandLoader([mockCommandA]);
|
||||
const loader2 = new MockCommandLoader([mockCommandB]);
|
||||
|
||||
await CommandService.create([loader1, loader2], signal);
|
||||
|
||||
expect(loader1.loadCommands).toHaveBeenCalledTimes(1);
|
||||
expect(loader1.loadCommands).toHaveBeenCalledWith(signal);
|
||||
expect(loader2.loadCommands).toHaveBeenCalledTimes(1);
|
||||
expect(loader2.loadCommands).toHaveBeenCalledWith(signal);
|
||||
});
|
||||
|
||||
it('should rename extension commands when they conflict', async () => {
|
||||
const builtinCommand = createMockCommand('deploy', CommandKind.BUILT_IN);
|
||||
const userCommand = createMockCommand('sync', CommandKind.FILE);
|
||||
const extensionCommand1 = {
|
||||
...createMockCommand('deploy', CommandKind.FILE),
|
||||
extensionName: 'firebase',
|
||||
description: '[firebase] Deploy to Firebase',
|
||||
};
|
||||
const extensionCommand2 = {
|
||||
...createMockCommand('sync', CommandKind.FILE),
|
||||
extensionName: 'git-helper',
|
||||
description: '[git-helper] Sync with remote',
|
||||
};
|
||||
|
||||
const mockLoader1 = new MockCommandLoader([builtinCommand]);
|
||||
const mockLoader2 = new MockCommandLoader([
|
||||
userCommand,
|
||||
extensionCommand1,
|
||||
extensionCommand2,
|
||||
]);
|
||||
|
||||
const service = await CommandService.create(
|
||||
[mockLoader1, mockLoader2],
|
||||
new AbortController().signal,
|
||||
);
|
||||
|
||||
const commands = service.getCommands();
|
||||
expect(commands).toHaveLength(4);
|
||||
|
||||
// Built-in command keeps original name
|
||||
const deployBuiltin = commands.find(
|
||||
(cmd) => cmd.name === 'deploy' && !cmd.extensionName,
|
||||
);
|
||||
expect(deployBuiltin).toBeDefined();
|
||||
expect(deployBuiltin?.kind).toBe(CommandKind.BUILT_IN);
|
||||
|
||||
// Extension command conflicting with built-in gets renamed
|
||||
const deployExtension = commands.find(
|
||||
(cmd) => cmd.name === 'firebase.deploy',
|
||||
);
|
||||
expect(deployExtension).toBeDefined();
|
||||
expect(deployExtension?.extensionName).toBe('firebase');
|
||||
|
||||
// User command keeps original name
|
||||
const syncUser = commands.find(
|
||||
(cmd) => cmd.name === 'sync' && !cmd.extensionName,
|
||||
);
|
||||
expect(syncUser).toBeDefined();
|
||||
expect(syncUser?.kind).toBe(CommandKind.FILE);
|
||||
|
||||
// Extension command conflicting with user command gets renamed
|
||||
const syncExtension = commands.find(
|
||||
(cmd) => cmd.name === 'git-helper.sync',
|
||||
);
|
||||
expect(syncExtension).toBeDefined();
|
||||
expect(syncExtension?.extensionName).toBe('git-helper');
|
||||
});
|
||||
|
||||
it('should handle user/project command override correctly', async () => {
|
||||
const builtinCommand = createMockCommand('help', CommandKind.BUILT_IN);
|
||||
const userCommand = createMockCommand('help', CommandKind.FILE);
|
||||
const projectCommand = createMockCommand('deploy', CommandKind.FILE);
|
||||
const userDeployCommand = createMockCommand('deploy', CommandKind.FILE);
|
||||
|
||||
const mockLoader1 = new MockCommandLoader([builtinCommand]);
|
||||
const mockLoader2 = new MockCommandLoader([
|
||||
userCommand,
|
||||
userDeployCommand,
|
||||
projectCommand,
|
||||
]);
|
||||
|
||||
const service = await CommandService.create(
|
||||
[mockLoader1, mockLoader2],
|
||||
new AbortController().signal,
|
||||
);
|
||||
|
||||
const commands = service.getCommands();
|
||||
expect(commands).toHaveLength(2);
|
||||
|
||||
// User command overrides built-in
|
||||
const helpCommand = commands.find((cmd) => cmd.name === 'help');
|
||||
expect(helpCommand).toBeDefined();
|
||||
expect(helpCommand?.kind).toBe(CommandKind.FILE);
|
||||
|
||||
// Project command overrides user command (last wins)
|
||||
const deployCommand = commands.find((cmd) => cmd.name === 'deploy');
|
||||
expect(deployCommand).toBeDefined();
|
||||
expect(deployCommand?.kind).toBe(CommandKind.FILE);
|
||||
});
|
||||
|
||||
it('should handle secondary conflicts when renaming extension commands', async () => {
|
||||
// User has both /deploy and /gcp.deploy commands
|
||||
const userCommand1 = createMockCommand('deploy', CommandKind.FILE);
|
||||
const userCommand2 = createMockCommand('gcp.deploy', CommandKind.FILE);
|
||||
|
||||
// Extension also has a deploy command that will conflict with user's /deploy
|
||||
const extensionCommand = {
|
||||
...createMockCommand('deploy', CommandKind.FILE),
|
||||
extensionName: 'gcp',
|
||||
description: '[gcp] Deploy to Google Cloud',
|
||||
};
|
||||
|
||||
const mockLoader = new MockCommandLoader([
|
||||
userCommand1,
|
||||
userCommand2,
|
||||
extensionCommand,
|
||||
]);
|
||||
|
||||
const service = await CommandService.create(
|
||||
[mockLoader],
|
||||
new AbortController().signal,
|
||||
);
|
||||
|
||||
const commands = service.getCommands();
|
||||
expect(commands).toHaveLength(3);
|
||||
|
||||
// Original user command keeps its name
|
||||
const deployUser = commands.find(
|
||||
(cmd) => cmd.name === 'deploy' && !cmd.extensionName,
|
||||
);
|
||||
expect(deployUser).toBeDefined();
|
||||
|
||||
// User's dot notation command keeps its name
|
||||
const gcpDeployUser = commands.find(
|
||||
(cmd) => cmd.name === 'gcp.deploy' && !cmd.extensionName,
|
||||
);
|
||||
expect(gcpDeployUser).toBeDefined();
|
||||
|
||||
// Extension command gets renamed with suffix due to secondary conflict
|
||||
const deployExtension = commands.find(
|
||||
(cmd) => cmd.name === 'gcp.deploy1' && cmd.extensionName === 'gcp',
|
||||
);
|
||||
expect(deployExtension).toBeDefined();
|
||||
expect(deployExtension?.description).toBe('[gcp] Deploy to Google Cloud');
|
||||
});
|
||||
|
||||
it('should handle multiple secondary conflicts with incrementing suffixes', async () => {
|
||||
// User has /deploy, /gcp.deploy, and /gcp.deploy1
|
||||
const userCommand1 = createMockCommand('deploy', CommandKind.FILE);
|
||||
const userCommand2 = createMockCommand('gcp.deploy', CommandKind.FILE);
|
||||
const userCommand3 = createMockCommand('gcp.deploy1', CommandKind.FILE);
|
||||
|
||||
// Extension has a deploy command
|
||||
const extensionCommand = {
|
||||
...createMockCommand('deploy', CommandKind.FILE),
|
||||
extensionName: 'gcp',
|
||||
description: '[gcp] Deploy to Google Cloud',
|
||||
};
|
||||
|
||||
const mockLoader = new MockCommandLoader([
|
||||
userCommand1,
|
||||
userCommand2,
|
||||
userCommand3,
|
||||
extensionCommand,
|
||||
]);
|
||||
|
||||
const service = await CommandService.create(
|
||||
[mockLoader],
|
||||
new AbortController().signal,
|
||||
);
|
||||
|
||||
const commands = service.getCommands();
|
||||
expect(commands).toHaveLength(4);
|
||||
|
||||
// Extension command gets renamed with suffix 2 due to multiple conflicts
|
||||
const deployExtension = commands.find(
|
||||
(cmd) => cmd.name === 'gcp.deploy2' && cmd.extensionName === 'gcp',
|
||||
);
|
||||
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 pass the abort signal to all loaders', async () => {
|
||||
const controller = new AbortController();
|
||||
const loader = new MockCommandLoader([]);
|
||||
await CommandService.create([loader], controller.signal);
|
||||
expect(loader.loadCommands).toHaveBeenCalledWith(controller.signal);
|
||||
});
|
||||
});
|
||||
|
||||
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',
|
||||
};
|
||||
describe('conflict delegation', () => {
|
||||
it('should delegate conflict resolution to SlashCommandResolver', async () => {
|
||||
const builtin = createMockCommand('help', CommandKind.BUILT_IN);
|
||||
const user = createMockCommand('help', CommandKind.USER_FILE);
|
||||
|
||||
const mockLoader = new MockCommandLoader([
|
||||
extension1Command,
|
||||
extension2Command,
|
||||
]);
|
||||
const service = await CommandService.create(
|
||||
[new MockCommandLoader([builtin, user])],
|
||||
new AbortController().signal,
|
||||
);
|
||||
|
||||
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',
|
||||
}),
|
||||
},
|
||||
],
|
||||
expect(service.getCommands().map((c) => c.name)).toContain('help');
|
||||
expect(service.getCommands().map((c) => c.name)).toContain('user.help');
|
||||
expect(service.getConflicts()).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
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' }),
|
||||
}),
|
||||
]),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -6,16 +6,8 @@
|
||||
|
||||
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;
|
||||
}>;
|
||||
}
|
||||
import type { ICommandLoader, CommandConflict } from './types.js';
|
||||
import { SlashCommandResolver } from './SlashCommandResolver.js';
|
||||
|
||||
/**
|
||||
* Orchestrates the discovery and loading of all slash commands for the CLI.
|
||||
@@ -24,9 +16,9 @@ export interface CommandConflict {
|
||||
* with an array of `ICommandLoader` instances, each responsible for fetching
|
||||
* commands from a specific source (e.g., built-in code, local files).
|
||||
*
|
||||
* The CommandService is responsible for invoking these loaders, aggregating their
|
||||
* results, and resolving any name conflicts. This architecture allows the command
|
||||
* system to be extended with new sources without modifying the service itself.
|
||||
* It uses a delegating resolver to reconcile name conflicts, ensuring that
|
||||
* all commands are uniquely addressable via source-specific prefixes while
|
||||
* allowing built-in commands to retain their primary names.
|
||||
*/
|
||||
export class CommandService {
|
||||
/**
|
||||
@@ -42,96 +34,71 @@ export class CommandService {
|
||||
/**
|
||||
* Asynchronously creates and initializes a new CommandService instance.
|
||||
*
|
||||
* This factory method orchestrates the entire command loading process. It
|
||||
* runs all provided loaders in parallel, aggregates their results, handles
|
||||
* name conflicts for extension commands by renaming them, and then returns a
|
||||
* fully constructed `CommandService` instance.
|
||||
* This factory method orchestrates the loading process and delegates
|
||||
* conflict resolution to the SlashCommandResolver.
|
||||
*
|
||||
* Conflict resolution:
|
||||
* - Extension commands that conflict with existing commands are renamed to
|
||||
* `extensionName.commandName`
|
||||
* - Non-extension commands (built-in, user, project) override earlier commands
|
||||
* with the same name based on loader order
|
||||
*
|
||||
* @param loaders An array of objects that conform to the `ICommandLoader`
|
||||
* interface. Built-in commands should come first, followed by FileCommandLoader.
|
||||
* @param signal An AbortSignal to cancel the loading process.
|
||||
* @returns A promise that resolves to a new, fully initialized `CommandService` instance.
|
||||
* @param loaders An array of loaders to fetch commands from.
|
||||
* @param signal An AbortSignal to allow cancellation.
|
||||
* @returns A promise that resolves to a fully initialized CommandService.
|
||||
*/
|
||||
static async create(
|
||||
loaders: ICommandLoader[],
|
||||
signal: AbortSignal,
|
||||
): Promise<CommandService> {
|
||||
const allCommands = await this.loadAllCommands(loaders, signal);
|
||||
const { finalCommands, conflicts } =
|
||||
SlashCommandResolver.resolve(allCommands);
|
||||
|
||||
if (conflicts.length > 0) {
|
||||
this.emitConflictEvents(conflicts);
|
||||
}
|
||||
|
||||
return new CommandService(
|
||||
Object.freeze(finalCommands),
|
||||
Object.freeze(conflicts),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Invokes all loaders in parallel and flattens the results.
|
||||
*/
|
||||
private static async loadAllCommands(
|
||||
loaders: ICommandLoader[],
|
||||
signal: AbortSignal,
|
||||
): Promise<SlashCommand[]> {
|
||||
const results = await Promise.allSettled(
|
||||
loaders.map((loader) => loader.loadCommands(signal)),
|
||||
);
|
||||
|
||||
const allCommands: SlashCommand[] = [];
|
||||
const commands: SlashCommand[] = [];
|
||||
for (const result of results) {
|
||||
if (result.status === 'fulfilled') {
|
||||
allCommands.push(...result.value);
|
||||
commands.push(...result.value);
|
||||
} else {
|
||||
debugLogger.debug('A command loader failed:', result.reason);
|
||||
}
|
||||
}
|
||||
return commands;
|
||||
}
|
||||
|
||||
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;
|
||||
|
||||
// Keep trying until we find a name that doesn't conflict
|
||||
while (commandMap.has(renamedName)) {
|
||||
renamedName = `${cmd.extensionName}.${cmd.name}${suffix}`;
|
||||
suffix++;
|
||||
}
|
||||
|
||||
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, {
|
||||
...cmd,
|
||||
name: finalName,
|
||||
});
|
||||
}
|
||||
|
||||
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()));
|
||||
const finalConflicts = Object.freeze(conflicts);
|
||||
return new CommandService(finalCommands, finalConflicts);
|
||||
/**
|
||||
* Formats and emits telemetry for command conflicts.
|
||||
*/
|
||||
private static emitConflictEvents(conflicts: CommandConflict[]): void {
|
||||
coreEvents.emitSlashCommandConflicts(
|
||||
conflicts.flatMap((c) =>
|
||||
c.losers.map((l) => ({
|
||||
name: c.name,
|
||||
renamedTo: l.renamedTo,
|
||||
loserExtensionName: l.command.extensionName,
|
||||
winnerExtensionName: l.reason.extensionName,
|
||||
loserMcpServerName: l.command.mcpServerName,
|
||||
winnerMcpServerName: l.reason.mcpServerName,
|
||||
loserKind: l.command.kind,
|
||||
winnerKind: l.reason.kind,
|
||||
})),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -37,6 +37,7 @@ import { sanitizeForDisplay } from '../ui/utils/textUtils.js';
|
||||
|
||||
interface CommandDirectory {
|
||||
path: string;
|
||||
kind: CommandKind;
|
||||
extensionName?: string;
|
||||
extensionId?: string;
|
||||
}
|
||||
@@ -111,6 +112,7 @@ export class FileCommandLoader implements ICommandLoader {
|
||||
this.parseAndAdaptFile(
|
||||
path.join(dirInfo.path, file),
|
||||
dirInfo.path,
|
||||
dirInfo.kind,
|
||||
dirInfo.extensionName,
|
||||
dirInfo.extensionId,
|
||||
),
|
||||
@@ -151,10 +153,16 @@ export class FileCommandLoader implements ICommandLoader {
|
||||
const storage = this.config?.storage ?? new Storage(this.projectRoot);
|
||||
|
||||
// 1. User commands
|
||||
dirs.push({ path: Storage.getUserCommandsDir() });
|
||||
dirs.push({
|
||||
path: Storage.getUserCommandsDir(),
|
||||
kind: CommandKind.USER_FILE,
|
||||
});
|
||||
|
||||
// 2. Project commands (override user commands)
|
||||
dirs.push({ path: storage.getProjectCommandsDir() });
|
||||
// 2. Project commands
|
||||
dirs.push({
|
||||
path: storage.getProjectCommandsDir(),
|
||||
kind: CommandKind.WORKSPACE_FILE,
|
||||
});
|
||||
|
||||
// 3. Extension commands (processed last to detect all conflicts)
|
||||
if (this.config) {
|
||||
@@ -165,6 +173,7 @@ export class FileCommandLoader implements ICommandLoader {
|
||||
|
||||
const extensionCommandDirs = activeExtensions.map((ext) => ({
|
||||
path: path.join(ext.path, 'commands'),
|
||||
kind: CommandKind.EXTENSION_FILE,
|
||||
extensionName: ext.name,
|
||||
extensionId: ext.id,
|
||||
}));
|
||||
@@ -179,12 +188,14 @@ export class FileCommandLoader implements ICommandLoader {
|
||||
* Parses a single .toml file and transforms it into a SlashCommand object.
|
||||
* @param filePath The absolute path to the .toml file.
|
||||
* @param baseDir The root command directory for name calculation.
|
||||
* @param kind The CommandKind.
|
||||
* @param extensionName Optional extension name to prefix commands with.
|
||||
* @returns A promise resolving to a SlashCommand, or null if the file is invalid.
|
||||
*/
|
||||
private async parseAndAdaptFile(
|
||||
filePath: string,
|
||||
baseDir: string,
|
||||
kind: CommandKind,
|
||||
extensionName?: string,
|
||||
extensionId?: string,
|
||||
): Promise<SlashCommand | null> {
|
||||
@@ -286,7 +297,7 @@ export class FileCommandLoader implements ICommandLoader {
|
||||
return {
|
||||
name: baseCommandName,
|
||||
description,
|
||||
kind: CommandKind.FILE,
|
||||
kind,
|
||||
extensionName,
|
||||
extensionId,
|
||||
action: async (
|
||||
|
||||
@@ -44,6 +44,7 @@ export class McpPromptLoader implements ICommandLoader {
|
||||
name: commandName,
|
||||
description: prompt.description || `Invoke prompt ${prompt.name}`,
|
||||
kind: CommandKind.MCP_PROMPT,
|
||||
mcpServerName: serverName,
|
||||
autoExecute: !prompt.arguments || prompt.arguments.length === 0,
|
||||
subCommands: [
|
||||
{
|
||||
|
||||
@@ -0,0 +1,175 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2026 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { SlashCommandConflictHandler } from './SlashCommandConflictHandler.js';
|
||||
import {
|
||||
coreEvents,
|
||||
CoreEvent,
|
||||
type SlashCommandConflictsPayload,
|
||||
type SlashCommandConflict,
|
||||
} from '@google/gemini-cli-core';
|
||||
import { CommandKind } from '../ui/commands/types.js';
|
||||
|
||||
vi.mock('@google/gemini-cli-core', async (importOriginal) => {
|
||||
const actual =
|
||||
await importOriginal<typeof import('@google/gemini-cli-core')>();
|
||||
return {
|
||||
...actual,
|
||||
coreEvents: {
|
||||
on: vi.fn(),
|
||||
off: vi.fn(),
|
||||
emitFeedback: vi.fn(),
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
describe('SlashCommandConflictHandler', () => {
|
||||
let handler: SlashCommandConflictHandler;
|
||||
|
||||
/**
|
||||
* Helper to find and invoke the registered conflict event listener.
|
||||
*/
|
||||
const simulateEvent = (conflicts: SlashCommandConflict[]) => {
|
||||
const callback = vi
|
||||
.mocked(coreEvents.on)
|
||||
.mock.calls.find(
|
||||
(call) => call[0] === CoreEvent.SlashCommandConflicts,
|
||||
)![1] as (payload: SlashCommandConflictsPayload) => void;
|
||||
callback({ conflicts });
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
handler = new SlashCommandConflictHandler();
|
||||
handler.start();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
handler.stop();
|
||||
vi.clearAllMocks();
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it('should listen for conflict events on start', () => {
|
||||
expect(coreEvents.on).toHaveBeenCalledWith(
|
||||
CoreEvent.SlashCommandConflicts,
|
||||
expect.any(Function),
|
||||
);
|
||||
});
|
||||
|
||||
it('should display a descriptive message for a single extension conflict', () => {
|
||||
simulateEvent([
|
||||
{
|
||||
name: 'deploy',
|
||||
renamedTo: 'firebase.deploy',
|
||||
loserExtensionName: 'firebase',
|
||||
loserKind: CommandKind.EXTENSION_FILE,
|
||||
winnerKind: CommandKind.BUILT_IN,
|
||||
},
|
||||
]);
|
||||
|
||||
vi.advanceTimersByTime(600);
|
||||
|
||||
expect(coreEvents.emitFeedback).toHaveBeenCalledWith(
|
||||
'info',
|
||||
"Extension 'firebase' command '/deploy' was renamed to '/firebase.deploy' because it conflicts with built-in command.",
|
||||
);
|
||||
});
|
||||
|
||||
it('should display a descriptive message for a single MCP conflict', () => {
|
||||
simulateEvent([
|
||||
{
|
||||
name: 'pickle',
|
||||
renamedTo: 'test-server.pickle',
|
||||
loserMcpServerName: 'test-server',
|
||||
loserKind: CommandKind.MCP_PROMPT,
|
||||
winnerExtensionName: 'pickle-rick',
|
||||
winnerKind: CommandKind.EXTENSION_FILE,
|
||||
},
|
||||
]);
|
||||
|
||||
vi.advanceTimersByTime(600);
|
||||
|
||||
expect(coreEvents.emitFeedback).toHaveBeenCalledWith(
|
||||
'info',
|
||||
"MCP server 'test-server' command '/pickle' was renamed to '/test-server.pickle' because it conflicts with extension 'pickle-rick' command.",
|
||||
);
|
||||
});
|
||||
|
||||
it('should group multiple conflicts for the same command name', () => {
|
||||
simulateEvent([
|
||||
{
|
||||
name: 'launch',
|
||||
renamedTo: 'user.launch',
|
||||
loserKind: CommandKind.USER_FILE,
|
||||
winnerKind: CommandKind.WORKSPACE_FILE,
|
||||
},
|
||||
{
|
||||
name: 'launch',
|
||||
renamedTo: 'workspace.launch',
|
||||
loserKind: CommandKind.WORKSPACE_FILE,
|
||||
winnerKind: CommandKind.USER_FILE,
|
||||
},
|
||||
]);
|
||||
|
||||
vi.advanceTimersByTime(600);
|
||||
|
||||
expect(coreEvents.emitFeedback).toHaveBeenCalledWith(
|
||||
'info',
|
||||
`Conflicts detected for command '/launch':
|
||||
- User command '/launch' was renamed to '/user.launch'
|
||||
- Workspace command '/launch' was renamed to '/workspace.launch'`,
|
||||
);
|
||||
});
|
||||
|
||||
it('should debounce multiple events within the flush window', () => {
|
||||
simulateEvent([
|
||||
{
|
||||
name: 'a',
|
||||
renamedTo: 'user.a',
|
||||
loserKind: CommandKind.USER_FILE,
|
||||
winnerKind: CommandKind.BUILT_IN,
|
||||
},
|
||||
]);
|
||||
|
||||
vi.advanceTimersByTime(200);
|
||||
|
||||
simulateEvent([
|
||||
{
|
||||
name: 'b',
|
||||
renamedTo: 'user.b',
|
||||
loserKind: CommandKind.USER_FILE,
|
||||
winnerKind: CommandKind.BUILT_IN,
|
||||
},
|
||||
]);
|
||||
|
||||
vi.advanceTimersByTime(600);
|
||||
|
||||
// Should emit two feedbacks (one for each unique command name)
|
||||
expect(coreEvents.emitFeedback).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('should deduplicate already notified conflicts', () => {
|
||||
const conflict = {
|
||||
name: 'deploy',
|
||||
renamedTo: 'firebase.deploy',
|
||||
loserExtensionName: 'firebase',
|
||||
loserKind: CommandKind.EXTENSION_FILE,
|
||||
winnerKind: CommandKind.BUILT_IN,
|
||||
};
|
||||
|
||||
simulateEvent([conflict]);
|
||||
vi.advanceTimersByTime(600);
|
||||
expect(coreEvents.emitFeedback).toHaveBeenCalledTimes(1);
|
||||
|
||||
vi.mocked(coreEvents.emitFeedback).mockClear();
|
||||
|
||||
simulateEvent([conflict]);
|
||||
vi.advanceTimersByTime(600);
|
||||
expect(coreEvents.emitFeedback).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -8,10 +8,20 @@ import {
|
||||
coreEvents,
|
||||
CoreEvent,
|
||||
type SlashCommandConflictsPayload,
|
||||
type SlashCommandConflict,
|
||||
} from '@google/gemini-cli-core';
|
||||
import { CommandKind } from '../ui/commands/types.js';
|
||||
|
||||
/**
|
||||
* Handles slash command conflict events and provides user feedback.
|
||||
*
|
||||
* This handler batches multiple conflict events into a single notification
|
||||
* block per command name to avoid UI clutter during startup or incremental loading.
|
||||
*/
|
||||
export class SlashCommandConflictHandler {
|
||||
private notifiedConflicts = new Set<string>();
|
||||
private pendingConflicts: SlashCommandConflict[] = [];
|
||||
private flushTimeout: ReturnType<typeof setTimeout> | null = null;
|
||||
|
||||
constructor() {
|
||||
this.handleConflicts = this.handleConflicts.bind(this);
|
||||
@@ -23,11 +33,18 @@ export class SlashCommandConflictHandler {
|
||||
|
||||
stop() {
|
||||
coreEvents.off(CoreEvent.SlashCommandConflicts, this.handleConflicts);
|
||||
if (this.flushTimeout) {
|
||||
clearTimeout(this.flushTimeout);
|
||||
this.flushTimeout = null;
|
||||
}
|
||||
}
|
||||
|
||||
private handleConflicts(payload: SlashCommandConflictsPayload) {
|
||||
const newConflicts = payload.conflicts.filter((c) => {
|
||||
const key = `${c.name}:${c.loserExtensionName}`;
|
||||
// Use a unique key to prevent duplicate notifications for the same conflict
|
||||
const sourceId =
|
||||
c.loserExtensionName || c.loserMcpServerName || c.loserKind;
|
||||
const key = `${c.name}:${sourceId}:${c.renamedTo}`;
|
||||
if (this.notifiedConflicts.has(key)) {
|
||||
return false;
|
||||
}
|
||||
@@ -36,19 +53,119 @@ export class SlashCommandConflictHandler {
|
||||
});
|
||||
|
||||
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');
|
||||
this.pendingConflicts.push(...newConflicts);
|
||||
this.scheduleFlush();
|
||||
}
|
||||
}
|
||||
|
||||
coreEvents.emitFeedback(
|
||||
'info',
|
||||
`Command conflicts detected:\n${conflictMessages}`,
|
||||
);
|
||||
private scheduleFlush() {
|
||||
if (this.flushTimeout) {
|
||||
clearTimeout(this.flushTimeout);
|
||||
}
|
||||
// Use a trailing debounce to capture staggered reloads during startup
|
||||
this.flushTimeout = setTimeout(() => this.flush(), 500);
|
||||
}
|
||||
|
||||
private flush() {
|
||||
this.flushTimeout = null;
|
||||
const conflicts = [...this.pendingConflicts];
|
||||
this.pendingConflicts = [];
|
||||
|
||||
if (conflicts.length === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Group conflicts by their original command name
|
||||
const grouped = new Map<string, SlashCommandConflict[]>();
|
||||
for (const c of conflicts) {
|
||||
const list = grouped.get(c.name) ?? [];
|
||||
list.push(c);
|
||||
grouped.set(c.name, list);
|
||||
}
|
||||
|
||||
for (const [name, commandConflicts] of grouped) {
|
||||
if (commandConflicts.length > 1) {
|
||||
this.emitGroupedFeedback(name, commandConflicts);
|
||||
} else {
|
||||
this.emitSingleFeedback(commandConflicts[0]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Emits a grouped notification for multiple conflicts sharing the same name.
|
||||
*/
|
||||
private emitGroupedFeedback(
|
||||
name: string,
|
||||
conflicts: SlashCommandConflict[],
|
||||
): void {
|
||||
const messages = conflicts
|
||||
.map((c) => {
|
||||
const source = this.getSourceDescription(
|
||||
c.loserExtensionName,
|
||||
c.loserKind,
|
||||
c.loserMcpServerName,
|
||||
);
|
||||
return `- ${this.capitalize(source)} '/${c.name}' was renamed to '/${c.renamedTo}'`;
|
||||
})
|
||||
.join('\n');
|
||||
|
||||
coreEvents.emitFeedback(
|
||||
'info',
|
||||
`Conflicts detected for command '/${name}':\n${messages}`,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Emits a descriptive notification for a single command conflict.
|
||||
*/
|
||||
private emitSingleFeedback(c: SlashCommandConflict): void {
|
||||
const loserSource = this.getSourceDescription(
|
||||
c.loserExtensionName,
|
||||
c.loserKind,
|
||||
c.loserMcpServerName,
|
||||
);
|
||||
const winnerSource = this.getSourceDescription(
|
||||
c.winnerExtensionName,
|
||||
c.winnerKind,
|
||||
c.winnerMcpServerName,
|
||||
);
|
||||
|
||||
coreEvents.emitFeedback(
|
||||
'info',
|
||||
`${this.capitalize(loserSource)} '/${c.name}' was renamed to '/${c.renamedTo}' because it conflicts with ${winnerSource}.`,
|
||||
);
|
||||
}
|
||||
|
||||
private capitalize(s: string): string {
|
||||
return s.charAt(0).toUpperCase() + s.slice(1);
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns a human-readable description of a command's source.
|
||||
*/
|
||||
private getSourceDescription(
|
||||
extensionName?: string,
|
||||
kind?: string,
|
||||
mcpServerName?: string,
|
||||
): string {
|
||||
switch (kind) {
|
||||
case CommandKind.EXTENSION_FILE:
|
||||
return extensionName
|
||||
? `extension '${extensionName}' command`
|
||||
: 'extension command';
|
||||
case CommandKind.MCP_PROMPT:
|
||||
return mcpServerName
|
||||
? `MCP server '${mcpServerName}' command`
|
||||
: 'MCP server command';
|
||||
case CommandKind.USER_FILE:
|
||||
return 'user command';
|
||||
case CommandKind.WORKSPACE_FILE:
|
||||
return 'workspace command';
|
||||
case CommandKind.BUILT_IN:
|
||||
return 'built-in command';
|
||||
default:
|
||||
return 'existing command';
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,177 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2026 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { SlashCommandResolver } from './SlashCommandResolver.js';
|
||||
import { CommandKind, type SlashCommand } from '../ui/commands/types.js';
|
||||
|
||||
const createMockCommand = (name: string, kind: CommandKind): SlashCommand => ({
|
||||
name,
|
||||
description: `Description for ${name}`,
|
||||
kind,
|
||||
action: vi.fn(),
|
||||
});
|
||||
|
||||
describe('SlashCommandResolver', () => {
|
||||
describe('resolve', () => {
|
||||
it('should return all commands when there are no conflicts', () => {
|
||||
const cmdA = createMockCommand('a', CommandKind.BUILT_IN);
|
||||
const cmdB = createMockCommand('b', CommandKind.USER_FILE);
|
||||
|
||||
const { finalCommands, conflicts } = SlashCommandResolver.resolve([
|
||||
cmdA,
|
||||
cmdB,
|
||||
]);
|
||||
|
||||
expect(finalCommands).toHaveLength(2);
|
||||
expect(conflicts).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should rename extension commands when they conflict with built-in', () => {
|
||||
const builtin = createMockCommand('deploy', CommandKind.BUILT_IN);
|
||||
const extension = {
|
||||
...createMockCommand('deploy', CommandKind.EXTENSION_FILE),
|
||||
extensionName: 'firebase',
|
||||
};
|
||||
|
||||
const { finalCommands, conflicts } = SlashCommandResolver.resolve([
|
||||
builtin,
|
||||
extension,
|
||||
]);
|
||||
|
||||
expect(finalCommands.map((c) => c.name)).toContain('deploy');
|
||||
expect(finalCommands.map((c) => c.name)).toContain('firebase.deploy');
|
||||
expect(conflicts).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('should prefix both user and workspace commands when they conflict', () => {
|
||||
const userCmd = createMockCommand('sync', CommandKind.USER_FILE);
|
||||
const workspaceCmd = createMockCommand(
|
||||
'sync',
|
||||
CommandKind.WORKSPACE_FILE,
|
||||
);
|
||||
|
||||
const { finalCommands, conflicts } = SlashCommandResolver.resolve([
|
||||
userCmd,
|
||||
workspaceCmd,
|
||||
]);
|
||||
|
||||
const names = finalCommands.map((c) => c.name);
|
||||
expect(names).not.toContain('sync');
|
||||
expect(names).toContain('user.sync');
|
||||
expect(names).toContain('workspace.sync');
|
||||
expect(conflicts).toHaveLength(1);
|
||||
expect(conflicts[0].losers).toHaveLength(2); // Both are considered losers
|
||||
});
|
||||
|
||||
it('should prefix file commands but keep built-in names during conflicts', () => {
|
||||
const builtin = createMockCommand('help', CommandKind.BUILT_IN);
|
||||
const user = createMockCommand('help', CommandKind.USER_FILE);
|
||||
|
||||
const { finalCommands } = SlashCommandResolver.resolve([builtin, user]);
|
||||
|
||||
const names = finalCommands.map((c) => c.name);
|
||||
expect(names).toContain('help');
|
||||
expect(names).toContain('user.help');
|
||||
});
|
||||
|
||||
it('should prefix both commands when MCP and user file conflict', () => {
|
||||
const mcp = {
|
||||
...createMockCommand('test', CommandKind.MCP_PROMPT),
|
||||
mcpServerName: 'test-server',
|
||||
};
|
||||
const user = createMockCommand('test', CommandKind.USER_FILE);
|
||||
|
||||
const { finalCommands } = SlashCommandResolver.resolve([mcp, user]);
|
||||
|
||||
const names = finalCommands.map((c) => c.name);
|
||||
expect(names).not.toContain('test');
|
||||
expect(names).toContain('test-server.test');
|
||||
expect(names).toContain('user.test');
|
||||
});
|
||||
|
||||
it('should prefix MCP commands with server name when they conflict with built-in', () => {
|
||||
const builtin = createMockCommand('help', CommandKind.BUILT_IN);
|
||||
const mcp = {
|
||||
...createMockCommand('help', CommandKind.MCP_PROMPT),
|
||||
mcpServerName: 'test-server',
|
||||
};
|
||||
|
||||
const { finalCommands } = SlashCommandResolver.resolve([builtin, mcp]);
|
||||
|
||||
const names = finalCommands.map((c) => c.name);
|
||||
expect(names).toContain('help');
|
||||
expect(names).toContain('test-server.help');
|
||||
});
|
||||
|
||||
it('should prefix both MCP commands when they conflict with each other', () => {
|
||||
const mcp1 = {
|
||||
...createMockCommand('test', CommandKind.MCP_PROMPT),
|
||||
mcpServerName: 'server1',
|
||||
};
|
||||
const mcp2 = {
|
||||
...createMockCommand('test', CommandKind.MCP_PROMPT),
|
||||
mcpServerName: 'server2',
|
||||
};
|
||||
|
||||
const { finalCommands } = SlashCommandResolver.resolve([mcp1, mcp2]);
|
||||
|
||||
const names = finalCommands.map((c) => c.name);
|
||||
expect(names).not.toContain('test');
|
||||
expect(names).toContain('server1.test');
|
||||
expect(names).toContain('server2.test');
|
||||
});
|
||||
|
||||
it('should favor the last built-in command silently during conflicts', () => {
|
||||
const builtin1 = {
|
||||
...createMockCommand('help', CommandKind.BUILT_IN),
|
||||
description: 'first',
|
||||
};
|
||||
const builtin2 = {
|
||||
...createMockCommand('help', CommandKind.BUILT_IN),
|
||||
description: 'second',
|
||||
};
|
||||
|
||||
const { finalCommands } = SlashCommandResolver.resolve([
|
||||
builtin1,
|
||||
builtin2,
|
||||
]);
|
||||
|
||||
expect(finalCommands).toHaveLength(1);
|
||||
expect(finalCommands[0].description).toBe('second');
|
||||
});
|
||||
|
||||
it('should fallback to numeric suffixes when both prefix and kind-based prefix are missing', () => {
|
||||
const cmd1 = createMockCommand('test', CommandKind.BUILT_IN);
|
||||
const cmd2 = {
|
||||
...createMockCommand('test', 'unknown' as CommandKind),
|
||||
};
|
||||
|
||||
const { finalCommands } = SlashCommandResolver.resolve([cmd1, cmd2]);
|
||||
|
||||
const names = finalCommands.map((c) => c.name);
|
||||
expect(names).toContain('test');
|
||||
expect(names).toContain('test1');
|
||||
});
|
||||
|
||||
it('should apply numeric suffixes when renames also conflict', () => {
|
||||
const user1 = createMockCommand('deploy', CommandKind.USER_FILE);
|
||||
const user2 = createMockCommand('gcp.deploy', CommandKind.USER_FILE);
|
||||
const extension = {
|
||||
...createMockCommand('deploy', CommandKind.EXTENSION_FILE),
|
||||
extensionName: 'gcp',
|
||||
};
|
||||
|
||||
const { finalCommands } = SlashCommandResolver.resolve([
|
||||
user1,
|
||||
user2,
|
||||
extension,
|
||||
]);
|
||||
|
||||
expect(finalCommands.find((c) => c.name === 'gcp.deploy1')).toBeDefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,213 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2026 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import type { SlashCommand } from '../ui/commands/types.js';
|
||||
import { CommandKind } from '../ui/commands/types.js';
|
||||
import type { CommandConflict } from './types.js';
|
||||
|
||||
/**
|
||||
* Internal registry to track commands and conflicts during resolution.
|
||||
*/
|
||||
class CommandRegistry {
|
||||
readonly commandMap = new Map<string, SlashCommand>();
|
||||
readonly conflictsMap = new Map<string, CommandConflict>();
|
||||
readonly firstEncounters = new Map<string, SlashCommand>();
|
||||
|
||||
get finalCommands(): SlashCommand[] {
|
||||
return Array.from(this.commandMap.values());
|
||||
}
|
||||
|
||||
get conflicts(): CommandConflict[] {
|
||||
return Array.from(this.conflictsMap.values());
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves name conflicts among slash commands.
|
||||
*
|
||||
* Rules:
|
||||
* 1. Built-in commands always keep the original name.
|
||||
* 2. All other types are prefixed with their source name (e.g. user.name).
|
||||
* 3. If multiple non-built-in commands conflict, all of them are renamed.
|
||||
*/
|
||||
export class SlashCommandResolver {
|
||||
/**
|
||||
* Orchestrates conflict resolution by applying renaming rules to ensures
|
||||
* every command has a unique name.
|
||||
*/
|
||||
static resolve(allCommands: SlashCommand[]): {
|
||||
finalCommands: SlashCommand[];
|
||||
conflicts: CommandConflict[];
|
||||
} {
|
||||
const registry = new CommandRegistry();
|
||||
|
||||
for (const cmd of allCommands) {
|
||||
const originalName = cmd.name;
|
||||
let finalName = originalName;
|
||||
|
||||
if (registry.firstEncounters.has(originalName)) {
|
||||
// We've already seen a command with this name, so resolve the conflict.
|
||||
finalName = this.handleConflict(cmd, registry);
|
||||
} else {
|
||||
// Track the first claimant to report them as the conflict reason later.
|
||||
registry.firstEncounters.set(originalName, cmd);
|
||||
}
|
||||
|
||||
// Store under final name, ensuring the command object reflects it.
|
||||
registry.commandMap.set(finalName, {
|
||||
...cmd,
|
||||
name: finalName,
|
||||
});
|
||||
}
|
||||
|
||||
return {
|
||||
finalCommands: registry.finalCommands,
|
||||
conflicts: registry.conflicts,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves a name collision by deciding which command keeps the name and which is renamed.
|
||||
*
|
||||
* @param incoming The command currently being processed that has a name collision.
|
||||
* @param registry The internal state of the resolution process.
|
||||
* @returns The final name to be assigned to the `incoming` command.
|
||||
*/
|
||||
private static handleConflict(
|
||||
incoming: SlashCommand,
|
||||
registry: CommandRegistry,
|
||||
): string {
|
||||
const collidingName = incoming.name;
|
||||
const originalClaimant = registry.firstEncounters.get(collidingName)!;
|
||||
|
||||
// Incoming built-in takes priority. Prefix any existing owner.
|
||||
if (incoming.kind === CommandKind.BUILT_IN) {
|
||||
this.prefixExistingCommand(collidingName, incoming, registry);
|
||||
return collidingName;
|
||||
}
|
||||
|
||||
// Incoming non-built-in is renamed to its source-prefixed version.
|
||||
const renamedName = this.getRenamedName(
|
||||
incoming.name,
|
||||
this.getPrefix(incoming),
|
||||
registry.commandMap,
|
||||
);
|
||||
this.trackConflict(
|
||||
registry.conflictsMap,
|
||||
collidingName,
|
||||
originalClaimant,
|
||||
incoming,
|
||||
renamedName,
|
||||
);
|
||||
|
||||
// Prefix current owner as well if it isn't a built-in.
|
||||
this.prefixExistingCommand(collidingName, incoming, registry);
|
||||
|
||||
return renamedName;
|
||||
}
|
||||
|
||||
/**
|
||||
* Safely renames the command currently occupying a name in the registry.
|
||||
*
|
||||
* @param name The name of the command to prefix.
|
||||
* @param reason The incoming command that is causing the prefixing.
|
||||
* @param registry The internal state of the resolution process.
|
||||
*/
|
||||
private static prefixExistingCommand(
|
||||
name: string,
|
||||
reason: SlashCommand,
|
||||
registry: CommandRegistry,
|
||||
): void {
|
||||
const currentOwner = registry.commandMap.get(name);
|
||||
|
||||
// Only non-built-in commands can be prefixed.
|
||||
if (!currentOwner || currentOwner.kind === CommandKind.BUILT_IN) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Determine the new name for the owner using its source prefix.
|
||||
const renamedName = this.getRenamedName(
|
||||
currentOwner.name,
|
||||
this.getPrefix(currentOwner),
|
||||
registry.commandMap,
|
||||
);
|
||||
|
||||
// Update the registry: remove the old name and add the owner under the new name.
|
||||
registry.commandMap.delete(name);
|
||||
const renamedOwner = { ...currentOwner, name: renamedName };
|
||||
registry.commandMap.set(renamedName, renamedOwner);
|
||||
|
||||
// Record the conflict so the user can be notified of the prefixing.
|
||||
this.trackConflict(
|
||||
registry.conflictsMap,
|
||||
name,
|
||||
reason,
|
||||
currentOwner,
|
||||
renamedName,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Generates a unique name using numeric suffixes if needed.
|
||||
*/
|
||||
private static getRenamedName(
|
||||
name: string,
|
||||
prefix: string | undefined,
|
||||
commandMap: Map<string, SlashCommand>,
|
||||
): string {
|
||||
const base = prefix ? `${prefix}.${name}` : name;
|
||||
let renamedName = base;
|
||||
let suffix = 1;
|
||||
|
||||
while (commandMap.has(renamedName)) {
|
||||
renamedName = `${base}${suffix}`;
|
||||
suffix++;
|
||||
}
|
||||
return renamedName;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns a suitable prefix for a conflicting command.
|
||||
*/
|
||||
private static getPrefix(cmd: SlashCommand): string | undefined {
|
||||
switch (cmd.kind) {
|
||||
case CommandKind.EXTENSION_FILE:
|
||||
return cmd.extensionName;
|
||||
case CommandKind.MCP_PROMPT:
|
||||
return cmd.mcpServerName;
|
||||
case CommandKind.USER_FILE:
|
||||
return 'user';
|
||||
case CommandKind.WORKSPACE_FILE:
|
||||
return 'workspace';
|
||||
default:
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Logs a conflict event.
|
||||
*/
|
||||
private static trackConflict(
|
||||
conflictsMap: Map<string, CommandConflict>,
|
||||
originalName: string,
|
||||
reason: SlashCommand,
|
||||
displacedCommand: SlashCommand,
|
||||
renamedTo: string,
|
||||
) {
|
||||
if (!conflictsMap.has(originalName)) {
|
||||
conflictsMap.set(originalName, {
|
||||
name: originalName,
|
||||
losers: [],
|
||||
});
|
||||
}
|
||||
|
||||
conflictsMap.get(originalName)!.losers.push({
|
||||
command: displacedCommand,
|
||||
renamedTo,
|
||||
reason,
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -22,3 +22,12 @@ export interface ICommandLoader {
|
||||
*/
|
||||
loadCommands(signal: AbortSignal): Promise<SlashCommand[]>;
|
||||
}
|
||||
|
||||
export interface CommandConflict {
|
||||
name: string;
|
||||
losers: Array<{
|
||||
command: SlashCommand;
|
||||
renamedTo: string;
|
||||
reason: SlashCommand;
|
||||
}>;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user