Agent Skills: Implement /skills reload (#15865)

packages/core/src/config/config.test.ts

@@ -33,6 +33,8 @@ import { RipGrepTool, canUseRipgrep } from '../tools/ripGrep.js';
 import { logRipgrepFallback } from '../telemetry/loggers.js';
 import { RipgrepFallbackEvent } from '../telemetry/types.js';
 import { ToolRegistry } from '../tools/tool-registry.js';
+import { ACTIVATE_SKILL_TOOL_NAME } from '../tools/tool-names.js';
+import type { SkillDefinition } from '../skills/skillLoader.js';
 import { DEFAULT_MODEL_CONFIGS } from './defaultModelConfigs.js';
 import {
   DEFAULT_GEMINI_MODEL,
@@ -57,6 +59,7 @@ vi.mock('fs', async (importOriginal) => {
 vi.mock('../tools/tool-registry', () => {
   const ToolRegistryMock = vi.fn();
   ToolRegistryMock.prototype.registerTool = vi.fn();
+  ToolRegistryMock.prototype.unregisterTool = vi.fn();
   ToolRegistryMock.prototype.discoverAllTools = vi.fn();
   ToolRegistryMock.prototype.sortTools = vi.fn();
   ToolRegistryMock.prototype.getAllTools = vi.fn(() => []); // Mock methods if needed
@@ -104,6 +107,7 @@ vi.mock('../core/client.js', () => ({
   GeminiClient: vi.fn().mockImplementation(() => ({
     initialize: vi.fn().mockResolvedValue(undefined),
     stripThoughtsFromHistory: vi.fn(),
+    isInitialized: vi.fn().mockReturnValue(false),
   })),
 }));
 
@@ -1978,4 +1982,104 @@ describe('Config JIT Initialization', () => {
     expect(ContextManager).not.toHaveBeenCalled();
     expect(config.getUserMemory()).toBe('Initial Memory');
   });
+
+  describe('reloadSkills', () => {
+    it('should refresh disabledSkills and re-register ActivateSkillTool when skills exist', async () => {
+      const mockOnReload = vi.fn().mockResolvedValue({
+        disabledSkills: ['skill2'],
+      });
+      const params: ConfigParameters = {
+        sessionId: 'test-session',
+        targetDir: '/tmp/test',
+        debugMode: false,
+        model: 'test-model',
+        cwd: '/tmp/test',
+        skillsSupport: true,
+        onReload: mockOnReload,
+      };
+
+      config = new Config(params);
+      await config.initialize();
+
+      const skillManager = config.getSkillManager();
+      const toolRegistry = config.getToolRegistry();
+
+      vi.spyOn(skillManager, 'discoverSkills').mockResolvedValue(undefined);
+      vi.spyOn(skillManager, 'setDisabledSkills');
+      vi.spyOn(toolRegistry, 'registerTool');
+      vi.spyOn(toolRegistry, 'unregisterTool');
+
+      const mockSkills = [{ name: 'skill1' }];
+      vi.spyOn(skillManager, 'getSkills').mockReturnValue(
+        mockSkills as SkillDefinition[],
+      );
+
+      await config.reloadSkills();
+
+      expect(mockOnReload).toHaveBeenCalled();
+      expect(skillManager.setDisabledSkills).toHaveBeenCalledWith(['skill2']);
+      expect(toolRegistry.registerTool).toHaveBeenCalled();
+      expect(toolRegistry.unregisterTool).not.toHaveBeenCalledWith(
+        ACTIVATE_SKILL_TOOL_NAME,
+      );
+    });
+
+    it('should unregister ActivateSkillTool when no skills exist after reload', async () => {
+      const params: ConfigParameters = {
+        sessionId: 'test-session',
+        targetDir: '/tmp/test',
+        debugMode: false,
+        model: 'test-model',
+        cwd: '/tmp/test',
+        skillsSupport: true,
+      };
+
+      config = new Config(params);
+      await config.initialize();
+
+      const skillManager = config.getSkillManager();
+      const toolRegistry = config.getToolRegistry();
+
+      vi.spyOn(skillManager, 'discoverSkills').mockResolvedValue(undefined);
+      vi.spyOn(toolRegistry, 'registerTool');
+      vi.spyOn(toolRegistry, 'unregisterTool');
+
+      vi.spyOn(skillManager, 'getSkills').mockReturnValue([]);
+
+      await config.reloadSkills();
+
+      expect(toolRegistry.unregisterTool).toHaveBeenCalledWith(
+        ACTIVATE_SKILL_TOOL_NAME,
+      );
+    });
+
+    it('should clear disabledSkills when onReload returns undefined for them', async () => {
+      const mockOnReload = vi.fn().mockResolvedValue({
+        disabledSkills: undefined,
+      });
+      const params: ConfigParameters = {
+        sessionId: 'test-session',
+        targetDir: '/tmp/test',
+        debugMode: false,
+        model: 'test-model',
+        cwd: '/tmp/test',
+        skillsSupport: true,
+        onReload: mockOnReload,
+      };
+
+      config = new Config(params);
+      // Initially set some disabled skills
+      // @ts-expect-error - accessing private
+      config.disabledSkills = ['skill1'];
+      await config.initialize();
+
+      const skillManager = config.getSkillManager();
+      vi.spyOn(skillManager, 'discoverSkills').mockResolvedValue(undefined);
+      vi.spyOn(skillManager, 'setDisabledSkills');
+
+      await config.reloadSkills();
+
+      expect(skillManager.setDisabledSkills).toHaveBeenCalledWith([]);
+    });
+  });
 });

packages/core/src/config/config.ts

@@ -356,6 +356,7 @@ export interface ConfigParameters {
   disabledSkills?: string[];
   experimentalJitContext?: boolean;
   onModelChange?: (model: string) => void;
+  onReload?: () => Promise<{ disabledSkills?: string[] }>;
 }
 
 export class Config {
@@ -479,10 +480,13 @@ export class Config {
   private experimentsPromise: Promise<void> | undefined;
   private hookSystem?: HookSystem;
   private readonly onModelChange: ((model: string) => void) | undefined;
+  private readonly onReload:
+    | (() => Promise<{ disabledSkills?: string[] }>)
+    | undefined;
 
   private readonly enableAgents: boolean;
   private readonly skillsSupport: boolean;
-  private readonly disabledSkills: string[];
+  private disabledSkills: string[];
 
   private readonly experimentalJitContext: boolean;
   private contextManager?: ContextManager;
@@ -643,6 +647,7 @@ export class Config {
     this.projectHooks = params.projectHooks;
     this.experiments = params.experiments;
     this.onModelChange = params.onModelChange;
+    this.onReload = params.onReload;
 
     if (params.contextFileName) {
       setGeminiMdFilename(params.contextFileName);
@@ -1520,6 +1525,38 @@ export class Config {
     return this.skillsSupport;
   }
 
+  /**
+   * Reloads skills by re-discovering them from extensions and local directories.
+   */
+  async reloadSkills(): Promise<void> {
+    if (!this.skillsSupport) {
+      return;
+    }
+
+    if (this.onReload) {
+      const refreshed = await this.onReload();
+      this.disabledSkills = refreshed.disabledSkills ?? [];
+    }
+
+    await this.getSkillManager().discoverSkills(
+      this.storage,
+      this.getExtensions(),
+    );
+    this.getSkillManager().setDisabledSkills(this.disabledSkills);
+
+    // Re-register ActivateSkillTool to update its schema with the newly discovered skills
+    if (this.getSkillManager().getSkills().length > 0) {
+      this.getToolRegistry().registerTool(
+        new ActivateSkillTool(this, this.messageBus),
+      );
+    } else {
+      this.getToolRegistry().unregisterTool(ActivateSkillTool.Name);
+    }
+
+    // Notify the client that system instructions might need updating
+    await this.updateSystemInstructionIfInitialized();
+  }
+
   isInteractive(): boolean {
     return this.interactive;
   }

packages/core/src/tools/tool-registry.ts

@@ -225,6 +225,15 @@ export class ToolRegistry {
     this.allKnownTools.set(tool.name, tool);
   }
 
+  /**
+   * Unregisters a tool definition by name.
+   *
+   * @param name - The name of the tool to unregister.
+   */
+  unregisterTool(name: string): void {
+    this.allKnownTools.delete(name);
+  }
+
   /**
    * Sorts tools as:
    * 1. Built in tools.

packages/core/src/utils/events.ts

@@ -74,6 +74,7 @@ export enum CoreEvent {
   Output = 'output',
   MemoryChanged = 'memory-changed',
   ExternalEditorClosed = 'external-editor-closed',
+  SettingsChanged = 'settings-changed',
 }
 
 export interface CoreEvents {
@@ -83,6 +84,7 @@ export interface CoreEvents {
   [CoreEvent.Output]: [OutputPayload];
   [CoreEvent.MemoryChanged]: [MemoryChangedPayload];
   [CoreEvent.ExternalEditorClosed]: never[];
+  [CoreEvent.SettingsChanged]: never[];
 }
 
 type EventBacklogItem = {
@@ -163,6 +165,13 @@ export class CoreEventEmitter extends EventEmitter<CoreEvents> {
     this.emit(CoreEvent.ModelChanged, payload);
   }
 
+  /**
+   * Notifies subscribers that settings have been modified.
+   */
+  emitSettingsChanged(): void {
+    this.emit(CoreEvent.SettingsChanged);
+  }
+
   /**
    * Flushes buffered messages. Call this immediately after primary UI listener
    * subscribes.