feat(cli): always prefix extension skills with colon separator (#23566)

This commit is contained in:
N. Taylor Mullen
2026-03-23 13:02:40 -07:00
committed by GitHub
parent 153f2630b9
commit 42a673a52c
2 changed files with 52 additions and 6 deletions

View File

@@ -43,7 +43,7 @@ describe('SlashCommandResolver', () => {
]);
expect(finalCommands.map((c) => c.name)).toContain('deploy');
expect(finalCommands.map((c) => c.name)).toContain('firebase.deploy');
expect(finalCommands.map((c) => c.name)).toContain('firebase:deploy');
expect(conflicts).toHaveLength(1);
});
@@ -159,7 +159,7 @@ describe('SlashCommandResolver', () => {
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 user2 = createMockCommand('gcp:deploy', CommandKind.USER_FILE);
const extension = {
...createMockCommand('deploy', CommandKind.EXTENSION_FILE),
extensionName: 'gcp',
@@ -171,7 +171,7 @@ describe('SlashCommandResolver', () => {
extension,
]);
expect(finalCommands.find((c) => c.name === 'gcp.deploy1')).toBeDefined();
expect(finalCommands.find((c) => c.name === 'gcp:deploy1')).toBeDefined();
});
it('should prefix skills with extension name when they conflict with built-in', () => {
@@ -185,7 +185,37 @@ describe('SlashCommandResolver', () => {
const names = finalCommands.map((c) => c.name);
expect(names).toContain('chat');
expect(names).toContain('google-workspace.chat');
expect(names).toContain('google-workspace:chat');
});
it('should ALWAYS prefix extension skills even if no conflict exists', () => {
const skill = {
...createMockCommand('chat', CommandKind.SKILL),
extensionName: 'google-workspace',
};
const { finalCommands } = SlashCommandResolver.resolve([skill]);
const names = finalCommands.map((c) => c.name);
expect(names).toContain('google-workspace:chat');
expect(names).not.toContain('chat');
});
it('should use numeric suffixes if prefixed skill names collide', () => {
const skill1 = {
...createMockCommand('chat', CommandKind.SKILL),
extensionName: 'google-workspace',
};
const skill2 = {
...createMockCommand('chat', CommandKind.SKILL),
extensionName: 'google-workspace',
};
const { finalCommands } = SlashCommandResolver.resolve([skill1, skill2]);
const names = finalCommands.map((c) => c.name);
expect(names).toContain('google-workspace:chat');
expect(names).toContain('google-workspace:chat1');
});
it('should NOT prefix skills with "skill" when extension name is missing', () => {

View File

@@ -47,7 +47,17 @@ export class SlashCommandResolver {
const originalName = cmd.name;
let finalName = originalName;
if (registry.firstEncounters.has(originalName)) {
const shouldAlwaysPrefix =
cmd.kind === CommandKind.SKILL && !!cmd.extensionName;
if (shouldAlwaysPrefix) {
finalName = this.getRenamedName(
originalName,
this.getPrefix(cmd),
registry.commandMap,
cmd.kind,
);
} else if (registry.firstEncounters.has(originalName)) {
// We've already seen a command with this name, so resolve the conflict.
finalName = this.handleConflict(cmd, registry);
} else {
@@ -93,6 +103,7 @@ export class SlashCommandResolver {
incoming.name,
this.getPrefix(incoming),
registry.commandMap,
incoming.kind,
);
this.trackConflict(
registry.conflictsMap,
@@ -132,6 +143,7 @@ export class SlashCommandResolver {
currentOwner.name,
this.getPrefix(currentOwner),
registry.commandMap,
currentOwner.kind,
);
// Update the registry: remove the old name and add the owner under the new name.
@@ -156,8 +168,12 @@ export class SlashCommandResolver {
name: string,
prefix: string | undefined,
commandMap: Map<string, SlashCommand>,
kind?: CommandKind,
): string {
const base = prefix ? `${prefix}.${name}` : name;
const isExtensionPrefix =
kind === CommandKind.SKILL || kind === CommandKind.EXTENSION_FILE;
const separator = isExtensionPrefix ? ':' : '.';
const base = prefix ? `${prefix}${separator}${name}` : name;
let renamedName = base;
let suffix = 1;