feat(ux): finalize slash commands and resolve skill conflict discovery

packages/core/src/skills/skillManager.ts

@@ -18,6 +18,7 @@ export class SkillManager {
   private skills: SkillDefinition[] = [];
   private activeSkillNames: Set<string> = new Set();
   private adminSkillsEnabled = true;
+  private pendingWarnings: Set<string> = new Set();
 
   /**
    * Clears all discovered skills.
@@ -50,6 +51,7 @@ export class SkillManager {
     isTrusted: boolean = false,
   ): Promise<void> {
     this.clearSkills();
+    this.pendingWarnings.clear();
 
     // 1. Built-in skills (lowest precedence)
     await this.discoverBuiltinSkills();
@@ -76,19 +78,27 @@ export class SkillManager {
       debugLogger.debug(
         'Workspace skills disabled because folder is not trusted.',
       );
-      return;
+    } else {
+      const projectSkills = await loadSkillsFromDir(
+        storage.getProjectSkillsDir(),
+      );
+      this.addSkillsWithPrecedence(projectSkills);
+
+      // 4.1 Workspace agent skills alias (.agents/skills)
+      const projectAgentSkills = await loadSkillsFromDir(
+        storage.getProjectAgentSkillsDir(),
+      );
+      this.addSkillsWithPrecedence(projectAgentSkills);
     }
 
-    const projectSkills = await loadSkillsFromDir(
-      storage.getProjectSkillsDir(),
-    );
-    this.addSkillsWithPrecedence(projectSkills);
+    this.emitPendingWarnings();
+  }
 
-    // 4.1 Workspace agent skills alias (.agents/skills)
-    const projectAgentSkills = await loadSkillsFromDir(
-      storage.getProjectAgentSkillsDir(),
-    );
-    this.addSkillsWithPrecedence(projectAgentSkills);
+  private emitPendingWarnings(): void {
+    for (const warning of this.pendingWarnings) {
+      coreEvents.emitFeedback('warning', warning);
+    }
+    this.pendingWarnings.clear();
   }
 
   /**
@@ -112,6 +122,7 @@ export class SkillManager {
    */
   addSkills(skills: SkillDefinition[]): void {
     this.addSkillsWithPrecedence(skills);
+    this.emitPendingWarnings();
   }
 
   private addSkillsWithPrecedence(newSkills: SkillDefinition[]): void {
@@ -127,8 +138,7 @@ export class SkillManager {
             `Skill "${newSkill.name}" from "${newSkill.location}" is overriding the built-in skill.`,
           );
         } else {
-          coreEvents.emitFeedback(
-            'warning',
+          this.pendingWarnings.add(
             `Skill conflict detected: "${newSkill.name}" from "${newSkill.location}" is overriding the same skill from "${existingSkill.location}".`,
           );
         }

packages/core/src/utils/extensionLoader.test.ts

@@ -263,10 +263,14 @@ describe('SimpleExtensionLoader', () => {
       vi.spyOn(loader, 'stopExtension');
       vi.spyOn(loader, 'startExtension');
       await loader.start(mockConfig);
+      mockSkillsReload.mockClear();
+
       await loader.restartExtension(activeExtension);
-      expect(loader.stopExtension).toHaveBeenCalledWith(activeExtension);
-      expect(loader.startExtension).toHaveBeenCalledWith(activeExtension);
-      expect(mockSkillsReload).toHaveBeenCalledTimes(2);
+      expect(loader.stopExtension).toHaveBeenCalledWith(activeExtension, true);
+      expect(loader.startExtension).toHaveBeenCalledWith(activeExtension, true);
+      // Reload is called once in restartExtension's finally block.
+      // (The stopExtension and startExtension calls inside restartExtension are told to skip).
+      expect(mockSkillsReload).toHaveBeenCalledOnce();
     });
   });
 });

packages/core/src/utils/extensionLoader.ts

@@ -46,7 +46,7 @@ export abstract class ExtensionLoader {
       await Promise.all(
         this.getExtensions()
           .filter((e) => e.isActive)
-          .map(this.startExtension.bind(this)),
+          .map((e) => this.startExtension(e)),
       );
     } finally {
       this.isStarting = false;
@@ -62,7 +62,10 @@ export abstract class ExtensionLoader {
    * go through `maybeStartExtension` which will only start the extension if
    * extension reloading is enabled and the `config` object is initialized.
    */
-  protected async startExtension(extension: GeminiCLIExtension) {
+  protected async startExtension(
+    extension: GeminiCLIExtension,
+    skipRefresh = false,
+  ) {
     if (!this.config) {
       throw new Error('Cannot call `startExtension` prior to calling `start`.');
     }
@@ -107,7 +110,9 @@ export abstract class ExtensionLoader {
         this.startingCount = 0;
         this.startCompletedCount = 0;
       }
-      await this.maybeRefreshMemories();
+      if (!skipRefresh) {
+        await this.maybeRefreshMemories();
+      }
     }
   }
 
@@ -169,7 +174,10 @@ export abstract class ExtensionLoader {
    * extension if extension reloading is enabled and the `config` object is
    * initialized.
    */
-  protected async stopExtension(extension: GeminiCLIExtension) {
+  protected async stopExtension(
+    extension: GeminiCLIExtension,
+    skipRefresh = false,
+  ) {
     if (!this.config) {
       throw new Error('Cannot call `stopExtension` prior to calling `start`.');
     }
@@ -221,7 +229,9 @@ export abstract class ExtensionLoader {
         this.stoppingCount = 0;
         this.stopCompletedCount = 0;
       }
-      await this.maybeRefreshMemories();
+      if (!skipRefresh) {
+        await this.maybeRefreshMemories();
+      }
     }
   }
 
@@ -239,8 +249,14 @@ export abstract class ExtensionLoader {
   }
 
   async restartExtension(extension: GeminiCLIExtension): Promise<void> {
-    await this.stopExtension(extension);
-    await this.startExtension(extension);
+    this.isStarting = true;
+    try {
+      await this.stopExtension(extension, true);
+      await this.startExtension(extension, true);
+    } finally {
+      this.isStarting = false;
+      await this.maybeRefreshMemories();
+    }
   }
 }