From 42a673a52ce0d881fe8348b48a951c2c8fda1a19 Mon Sep 17 00:00:00 2001
From: "N. Taylor Mullen" <ntaylormullen@google.com>
Date: Mon, 23 Mar 2026 13:02:40 -0700
Subject: [PATCH] feat(cli): always prefix extension skills with colon
 separator (#23566)

---
 .../src/services/SlashCommandResolver.test.ts | 38 +++++++++++++++++--
 .../cli/src/services/SlashCommandResolver.ts  | 20 +++++++++-
 2 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/packages/cli/src/services/SlashCommandResolver.test.ts b/packages/cli/src/services/SlashCommandResolver.test.ts
index 43d1c310a8..40e3b6f1d5 100644
--- a/packages/cli/src/services/SlashCommandResolver.test.ts
+++ b/packages/cli/src/services/SlashCommandResolver.test.ts
@@ -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', () => {
diff --git a/packages/cli/src/services/SlashCommandResolver.ts b/packages/cli/src/services/SlashCommandResolver.ts
index 4947e6545a..e956d6f566 100644
--- a/packages/cli/src/services/SlashCommandResolver.ts
+++ b/packages/cli/src/services/SlashCommandResolver.ts
@@ -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;