mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-11 06:31:01 -07:00
fix(cli): improve command conflict handling for skills
- Prioritize built-in commands in slashCommandProcessor to ensure they retain their intended names. - Add extensionName to SkillDefinition to track source extensions. - Update ExtensionManager to tag loaded skills with their source extension name. - Update SlashCommandResolver and SlashCommandConflictHandler to properly handle and describe conflicts involving skills. - Skill conflicts from extensions now produce clear notifications like "Extension 'google-workspace' skill '/chat' was renamed to '/google-workspace.chat'" instead of generic "Existing command" messages. Fixes built-in command rename issues observed on startup.
This commit is contained in:
@@ -891,9 +891,10 @@ Would you like to attempt to install via "git clone" instead?`,
|
||||
let skills = await loadSkillsFromDir(
|
||||
path.join(effectiveExtensionPath, 'skills'),
|
||||
);
|
||||
skills = skills.map((skill) =>
|
||||
recursivelyHydrateStrings(skill, hydrationContext),
|
||||
);
|
||||
skills = skills.map((skill) => ({
|
||||
...recursivelyHydrateStrings(skill, hydrationContext),
|
||||
extensionName: config.name,
|
||||
}));
|
||||
|
||||
let rules: PolicyRule[] | undefined;
|
||||
let checkers: SafetyCheckerRule[] | undefined;
|
||||
@@ -916,9 +917,10 @@ Would you like to attempt to install via "git clone" instead?`,
|
||||
const agentLoadResult = await loadAgentsFromDirectory(
|
||||
path.join(effectiveExtensionPath, 'agents'),
|
||||
);
|
||||
agentLoadResult.agents = agentLoadResult.agents.map((agent) =>
|
||||
recursivelyHydrateStrings(agent, hydrationContext),
|
||||
);
|
||||
agentLoadResult.agents = agentLoadResult.agents.map((agent) => ({
|
||||
...recursivelyHydrateStrings(agent, hydrationContext),
|
||||
extensionName: config.name,
|
||||
}));
|
||||
|
||||
// Log errors but don't fail the entire extension load
|
||||
for (const error of agentLoadResult.errors) {
|
||||
|
||||
@@ -122,4 +122,16 @@ describe('SkillCommandLoader', () => {
|
||||
const actionResult = (await commands[0].action!({} as any, '')) as any;
|
||||
expect(actionResult.toolArgs).toEqual({ name: 'my awesome skill' });
|
||||
});
|
||||
|
||||
it('should propagate extensionName to the generated slash command', async () => {
|
||||
const mockSkills = [
|
||||
{ name: 'skill1', description: 'desc', extensionName: 'ext1' },
|
||||
];
|
||||
mockSkillManager.getDisplayableSkills.mockReturnValue(mockSkills);
|
||||
|
||||
const loader = new SkillCommandLoader(mockConfig);
|
||||
const commands = await loader.loadCommands(new AbortController().signal);
|
||||
|
||||
expect(commands[0].extensionName).toBe('ext1');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -41,6 +41,7 @@ export class SkillCommandLoader implements ICommandLoader {
|
||||
description: skill.description || `Activate the ${skill.name} skill`,
|
||||
kind: CommandKind.SKILL,
|
||||
autoExecute: true,
|
||||
extensionName: skill.extensionName,
|
||||
action: async (_context, args) => ({
|
||||
type: 'tool',
|
||||
toolName: ACTIVATE_SKILL_TOOL_NAME,
|
||||
|
||||
@@ -172,4 +172,23 @@ describe('SlashCommandConflictHandler', () => {
|
||||
vi.advanceTimersByTime(600);
|
||||
expect(coreEvents.emitFeedback).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should display a descriptive message for a skill conflict', () => {
|
||||
simulateEvent([
|
||||
{
|
||||
name: 'chat',
|
||||
renamedTo: 'google-workspace.chat',
|
||||
loserExtensionName: 'google-workspace',
|
||||
loserKind: CommandKind.SKILL,
|
||||
winnerKind: CommandKind.BUILT_IN,
|
||||
},
|
||||
]);
|
||||
|
||||
vi.advanceTimersByTime(600);
|
||||
|
||||
expect(coreEvents.emitFeedback).toHaveBeenCalledWith(
|
||||
'info',
|
||||
"Extension 'google-workspace' skill '/chat' was renamed to '/google-workspace.chat' because it conflicts with built-in command.",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -154,6 +154,10 @@ export class SlashCommandConflictHandler {
|
||||
return extensionName
|
||||
? `extension '${extensionName}' command`
|
||||
: 'extension command';
|
||||
case CommandKind.SKILL:
|
||||
return extensionName
|
||||
? `extension '${extensionName}' skill`
|
||||
: 'skill command';
|
||||
case CommandKind.MCP_PROMPT:
|
||||
return mcpServerName
|
||||
? `MCP server '${mcpServerName}' command`
|
||||
|
||||
@@ -173,5 +173,30 @@ describe('SlashCommandResolver', () => {
|
||||
|
||||
expect(finalCommands.find((c) => c.name === 'gcp.deploy1')).toBeDefined();
|
||||
});
|
||||
|
||||
it('should prefix skills with extension name when they conflict with built-in', () => {
|
||||
const builtin = createMockCommand('chat', CommandKind.BUILT_IN);
|
||||
const skill = {
|
||||
...createMockCommand('chat', CommandKind.SKILL),
|
||||
extensionName: 'google-workspace',
|
||||
};
|
||||
|
||||
const { finalCommands } = SlashCommandResolver.resolve([builtin, skill]);
|
||||
|
||||
const names = finalCommands.map((c) => c.name);
|
||||
expect(names).toContain('chat');
|
||||
expect(names).toContain('google-workspace.chat');
|
||||
});
|
||||
|
||||
it('should NOT prefix skills with "skill" when extension name is missing', () => {
|
||||
const builtin = createMockCommand('chat', CommandKind.BUILT_IN);
|
||||
const skill = createMockCommand('chat', CommandKind.SKILL);
|
||||
|
||||
const { finalCommands } = SlashCommandResolver.resolve([builtin, skill]);
|
||||
|
||||
const names = finalCommands.map((c) => c.name);
|
||||
expect(names).toContain('chat');
|
||||
expect(names).toContain('chat1');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -175,6 +175,7 @@ export class SlashCommandResolver {
|
||||
private static getPrefix(cmd: SlashCommand): string | undefined {
|
||||
switch (cmd.kind) {
|
||||
case CommandKind.EXTENSION_FILE:
|
||||
case CommandKind.SKILL:
|
||||
return cmd.extensionName;
|
||||
case CommandKind.MCP_PROMPT:
|
||||
return cmd.mcpServerName;
|
||||
@@ -186,7 +187,6 @@ export class SlashCommandResolver {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Logs a conflict event.
|
||||
*/
|
||||
|
||||
@@ -325,9 +325,9 @@ export const useSlashCommandProcessor = (
|
||||
(async () => {
|
||||
const commandService = await CommandService.create(
|
||||
[
|
||||
new BuiltinCommandLoader(config),
|
||||
new SkillCommandLoader(config),
|
||||
new McpPromptLoader(config),
|
||||
new BuiltinCommandLoader(config),
|
||||
new FileCommandLoader(config),
|
||||
],
|
||||
controller.signal,
|
||||
|
||||
@@ -1061,6 +1061,10 @@ export const useGeminiStream = (
|
||||
'Response stopped due to unexpected tool call.',
|
||||
[FinishReason.IMAGE_PROHIBITED_CONTENT]:
|
||||
'Response stopped due to prohibited image content.',
|
||||
[FinishReason.IMAGE_RECITATION]:
|
||||
'Response stopped due to image recitation policy.',
|
||||
[FinishReason.IMAGE_OTHER]:
|
||||
'Response stopped due to other image issues.',
|
||||
[FinishReason.NO_IMAGE]:
|
||||
'Response stopped because no image was generated.',
|
||||
};
|
||||
|
||||
@@ -27,6 +27,8 @@ export interface SkillDefinition {
|
||||
disabled?: boolean;
|
||||
/** Whether the skill is a built-in skill. */
|
||||
isBuiltin?: boolean;
|
||||
/** The name of the extension that provided this skill, if any. */
|
||||
extensionName?: string;
|
||||
}
|
||||
|
||||
export const FRONTMATTER_REGEX =
|
||||
|
||||
Reference in New Issue
Block a user