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:
Taylor Mullen
2026-03-10 15:59:55 -07:00
parent 8bfa5b5054
commit bc8c52fc75
11 changed files with 76 additions and 9 deletions
+8 -6
View File
@@ -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');
});
});
});
@@ -174,6 +174,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;
@@ -185,7 +186,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,
+2 -1
View File
@@ -480,6 +480,7 @@ describe('oauth2', () => {
expect(fs.existsSync(googleAccountPath)).toBe(true);
if (fs.existsSync(googleAccountPath)) {
const cachedGoogleAccount = fs.readFileSync(googleAccountPath, 'utf-8');
expect(JSON.parse(cachedGoogleAccount)).toEqual({
active: 'test-user-code-account@gmail.com',
old: [],
@@ -1349,7 +1350,7 @@ describe('oauth2', () => {
let dataHandler: ((data: Buffer) => void) | undefined;
await vi.waitFor(() => {
const dataCall = stdinOnSpy.mock.calls.find(
(call: [string, ...unknown[]]) => call[0] === 'data',
(call: [string | symbol, ...unknown[]]) => call[0] === 'data',
);
dataHandler = dataCall?.[1] as ((data: Buffer) => void) | undefined;
if (!dataHandler) throw new Error('stdin handler not registered yet');
+2
View File
@@ -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 =
@@ -89,6 +89,7 @@ const mockHeapStatistics = {
total_global_handles_size: 8192,
used_global_handles_size: 4096,
external_memory: 2097152,
total_allocated_bytes: 31457280,
};
const mockHeapSpaceStatistics = [