From f0a039f7c07d7dae2f944a24b3786684cd514afd Mon Sep 17 00:00:00 2001 From: "N. Taylor Mullen" Date: Sat, 3 Jan 2026 16:24:36 -0800 Subject: [PATCH] Agent Skills: Unify Representation & Centralize Loading (#15833) --- .github/workflows/ci.yml | 2 +- .../cli/src/ui/commands/skillsCommand.test.ts | 32 ++- packages/cli/src/ui/commands/skillsCommand.ts | 2 + .../ui/components/views/SkillsList.test.tsx | 26 +- packages/cli/src/ui/types.ts | 9 +- packages/core/src/config/config.ts | 2 +- packages/core/src/index.ts | 2 + .../core/src/services/skillManager.test.ts | 237 ------------------ packages/core/src/services/skillManager.ts | 197 --------------- packages/core/src/skills/skillLoader.test.ts | 69 +++++ packages/core/src/skills/skillLoader.ts | 98 ++++++++ packages/core/src/skills/skillManager.test.ts | 88 +++++++ packages/core/src/skills/skillManager.ts | 97 +++++++ 13 files changed, 410 insertions(+), 451 deletions(-) delete mode 100644 packages/core/src/services/skillManager.test.ts delete mode 100644 packages/core/src/services/skillManager.ts create mode 100644 packages/core/src/skills/skillLoader.test.ts create mode 100644 packages/core/src/skills/skillLoader.ts create mode 100644 packages/core/src/skills/skillManager.test.ts create mode 100644 packages/core/src/skills/skillManager.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 08ac9ed52c..fa8ce717f1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -43,7 +43,7 @@ jobs: - id: 'merge-queue-ci-skipper' uses: 'cariad-tech/merge-queue-ci-skipper@1032489e59437862c90a08a2c92809c903883772' # ratchet:cariad-tech/merge-queue-ci-skipper@main with: - secret: '${{ secrets.GITHUB_TOKEN }}' + secret: '${{ secrets.GEMINI_CLI_ROBOT_GITHUB_PAT }}' lint: name: 'Lint' diff --git a/packages/cli/src/ui/commands/skillsCommand.test.ts b/packages/cli/src/ui/commands/skillsCommand.test.ts index 9d3b168b48..39339f8226 100644 --- a/packages/cli/src/ui/commands/skillsCommand.test.ts +++ b/packages/cli/src/ui/commands/skillsCommand.test.ts @@ -58,8 +58,20 @@ describe('skillsCommand', () => { expect.objectContaining({ type: MessageType.SKILLS_LIST, skills: [ - { name: 'skill1', description: 'desc1' }, - { name: 'skill2', description: 'desc2' }, + { + name: 'skill1', + description: 'desc1', + disabled: undefined, + location: '/loc1', + body: 'body1', + }, + { + name: 'skill2', + description: 'desc2', + disabled: undefined, + location: '/loc2', + body: 'body2', + }, ], showDescriptions: true, }), @@ -75,8 +87,20 @@ describe('skillsCommand', () => { expect.objectContaining({ type: MessageType.SKILLS_LIST, skills: [ - { name: 'skill1', description: 'desc1' }, - { name: 'skill2', description: 'desc2' }, + { + name: 'skill1', + description: 'desc1', + disabled: undefined, + location: '/loc1', + body: 'body1', + }, + { + name: 'skill2', + description: 'desc2', + disabled: undefined, + location: '/loc2', + body: 'body2', + }, ], showDescriptions: true, }), diff --git a/packages/cli/src/ui/commands/skillsCommand.ts b/packages/cli/src/ui/commands/skillsCommand.ts index 42fa0afc11..e3cbc568a1 100644 --- a/packages/cli/src/ui/commands/skillsCommand.ts +++ b/packages/cli/src/ui/commands/skillsCommand.ts @@ -45,6 +45,8 @@ async function listAction( name: skill.name, description: skill.description, disabled: skill.disabled, + location: skill.location, + body: skill.body, })), showDescriptions: useShowDescriptions, }; diff --git a/packages/cli/src/ui/components/views/SkillsList.test.tsx b/packages/cli/src/ui/components/views/SkillsList.test.tsx index 9d11ee241b..e04958cc9a 100644 --- a/packages/cli/src/ui/components/views/SkillsList.test.tsx +++ b/packages/cli/src/ui/components/views/SkillsList.test.tsx @@ -7,13 +7,31 @@ import { render } from '../../../test-utils/render.js'; import { describe, it, expect } from 'vitest'; import { SkillsList } from './SkillsList.js'; -import { type SkillDefinition } from '../../types.js'; +import { type SkillDefinition } from '@google/gemini-cli-core'; describe('SkillsList Component', () => { const mockSkills: SkillDefinition[] = [ - { name: 'skill1', description: 'description 1', disabled: false }, - { name: 'skill2', description: 'description 2', disabled: true }, - { name: 'skill3', description: 'description 3', disabled: false }, + { + name: 'skill1', + description: 'description 1', + disabled: false, + location: 'loc1', + body: 'body1', + }, + { + name: 'skill2', + description: 'description 2', + disabled: true, + location: 'loc2', + body: 'body2', + }, + { + name: 'skill3', + description: 'description 3', + disabled: false, + location: 'loc3', + body: 'body3', + }, ]; it('should render enabled and disabled skills separately', () => { diff --git a/packages/cli/src/ui/types.ts b/packages/cli/src/ui/types.ts index ede5ab5b84..c5947024f1 100644 --- a/packages/cli/src/ui/types.ts +++ b/packages/cli/src/ui/types.ts @@ -13,11 +13,12 @@ import type { ToolConfirmationOutcome, ToolResultDisplay, RetrieveUserQuotaResponse, + SkillDefinition, } from '@google/gemini-cli-core'; import type { PartListUnion } from '@google/genai'; import { type ReactNode } from 'react'; -export type { ThoughtSummary }; +export type { ThoughtSummary, SkillDefinition }; export enum AuthState { // Attempting to authenticate or re-authenticate @@ -206,12 +207,6 @@ export type HistoryItemToolsList = HistoryItemBase & { showDescriptions: boolean; }; -export interface SkillDefinition { - name: string; - description: string; - disabled?: boolean; -} - export type HistoryItemSkillsList = HistoryItemBase & { type: 'skills_list'; skills: SkillDefinition[]; diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 22640799e2..348e86b61e 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -97,7 +97,7 @@ import { DELEGATE_TO_AGENT_TOOL_NAME } from '../tools/tool-names.js'; import { getExperiments } from '../code_assist/experiments/experiments.js'; import { ExperimentFlags } from '../code_assist/experiments/flagNames.js'; import { debugLogger } from '../utils/debugLogger.js'; -import { SkillManager } from '../services/skillManager.js'; +import { SkillManager } from '../skills/skillManager.js'; import { startupProfiler } from '../telemetry/startupProfiler.js'; import { ApprovalMode } from '../policy/types.js'; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 0165fffcbf..246d5604e6 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -93,6 +93,8 @@ export * from './services/chatRecordingService.js'; export * from './services/fileSystemService.js'; export * from './services/sessionSummaryUtils.js'; export * from './services/contextManager.js'; +export * from './skills/skillManager.js'; +export * from './skills/skillLoader.js'; // Export IDE specific logic export * from './ide/ide-client.js'; diff --git a/packages/core/src/services/skillManager.test.ts b/packages/core/src/services/skillManager.test.ts deleted file mode 100644 index a7118605ca..0000000000 --- a/packages/core/src/services/skillManager.test.ts +++ /dev/null @@ -1,237 +0,0 @@ -/** - * @license - * Copyright 2025 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import * as fs from 'node:fs/promises'; -import * as os from 'node:os'; -import * as path from 'node:path'; -import { SkillManager } from './skillManager.js'; -import { Storage } from '../config/storage.js'; - -describe('SkillManager', () => { - let testRootDir: string; - - beforeEach(async () => { - testRootDir = await fs.mkdtemp( - path.join(os.tmpdir(), 'skill-manager-test-'), - ); - }); - - afterEach(async () => { - await fs.rm(testRootDir, { recursive: true, force: true }); - vi.restoreAllMocks(); - }); - - it('should discover skills with valid SKILL.md and frontmatter', async () => { - const skillDir = path.join(testRootDir, 'my-skill'); - await fs.mkdir(skillDir, { recursive: true }); - const skillFile = path.join(skillDir, 'SKILL.md'); - await fs.writeFile( - skillFile, - `--- -name: my-skill -description: A test skill ---- -# Instructions -Do something. -`, - ); - - const service = new SkillManager(); - const skills = await service.discoverSkillsInternal([testRootDir]); - - expect(skills).toHaveLength(1); - expect(skills[0].name).toBe('my-skill'); - expect(skills[0].description).toBe('A test skill'); - expect(skills[0].location).toBe(skillFile); - expect(skills[0].body).toBe('# Instructions\nDo something.'); - }); - - it('should ignore directories without SKILL.md', async () => { - const notASkillDir = path.join(testRootDir, 'not-a-skill'); - await fs.mkdir(notASkillDir, { recursive: true }); - - const service = new SkillManager(); - const skills = await service.discoverSkillsInternal([testRootDir]); - - expect(skills).toHaveLength(0); - }); - - it('should ignore SKILL.md without valid frontmatter', async () => { - const skillDir = path.join(testRootDir, 'invalid-skill'); - await fs.mkdir(skillDir, { recursive: true }); - const skillFile = path.join(skillDir, 'SKILL.md'); - await fs.writeFile(skillFile, '# No frontmatter here'); - - const service = new SkillManager(); - const skills = await service.discoverSkillsInternal([testRootDir]); - - expect(skills).toHaveLength(0); - }); - - it('should ignore SKILL.md with missing required frontmatter fields', async () => { - const skillDir = path.join(testRootDir, 'missing-fields'); - await fs.mkdir(skillDir, { recursive: true }); - const skillFile = path.join(skillDir, 'SKILL.md'); - await fs.writeFile( - skillFile, - `--- -name: missing-fields ---- -`, - ); - - const service = new SkillManager(); - const skills = await service.discoverSkillsInternal([testRootDir]); - - expect(skills).toHaveLength(0); - }); - - it('should handle multiple search paths', async () => { - const path1 = path.join(testRootDir, 'path1'); - const path2 = path.join(testRootDir, 'path2'); - await fs.mkdir(path1, { recursive: true }); - await fs.mkdir(path2, { recursive: true }); - - const skill1Dir = path.join(path1, 'skill1'); - await fs.mkdir(skill1Dir, { recursive: true }); - await fs.writeFile( - path.join(skill1Dir, 'SKILL.md'), - `--- -name: skill1 -description: Skill 1 ---- -`, - ); - - const skill2Dir = path.join(path2, 'skill2'); - await fs.mkdir(skill2Dir, { recursive: true }); - await fs.writeFile( - path.join(skill2Dir, 'SKILL.md'), - `--- -name: skill2 -description: Skill 2 ---- -`, - ); - - const service = new SkillManager(); - const skills = await service.discoverSkillsInternal([path1, path2]); - - expect(skills).toHaveLength(2); - expect(skills.map((s) => s.name).sort()).toEqual(['skill1', 'skill2']); - }); - - it('should deduplicate skills by name (last wins)', async () => { - const path1 = path.join(testRootDir, 'path1'); - const path2 = path.join(testRootDir, 'path2'); - await fs.mkdir(path1, { recursive: true }); - await fs.mkdir(path2, { recursive: true }); - - await fs.mkdir(path.join(path1, 'skill'), { recursive: true }); - await fs.writeFile( - path.join(path1, 'skill', 'SKILL.md'), - `--- -name: same-name -description: First ---- -`, - ); - - await fs.mkdir(path.join(path2, 'skill'), { recursive: true }); - await fs.writeFile( - path.join(path2, 'skill', 'SKILL.md'), - `--- -name: same-name -description: Second ---- -`, - ); - - const service = new SkillManager(); - // In our tiered discovery logic, we call discoverSkillsInternal for each tier - // and then add them with precedence. - const skills1 = await service.discoverSkillsInternal([path1]); - service['addSkillsWithPrecedence'](skills1); - const skills2 = await service.discoverSkillsInternal([path2]); - service['addSkillsWithPrecedence'](skills2); - - const skills = service.getSkills(); - expect(skills).toHaveLength(1); - expect(skills[0].description).toBe('Second'); - }); - - it('should discover skills from Storage with project precedence', async () => { - const userDir = path.join(testRootDir, 'user'); - const projectDir = path.join(testRootDir, 'project'); - await fs.mkdir(path.join(userDir, 'skill-a'), { recursive: true }); - await fs.mkdir(path.join(projectDir, 'skill-a'), { recursive: true }); - - await fs.writeFile( - path.join(userDir, 'skill-a', 'SKILL.md'), - `--- -name: skill-a -description: user-desc ---- -`, - ); - await fs.writeFile( - path.join(projectDir, 'skill-a', 'SKILL.md'), - `--- -name: skill-a -description: project-desc ---- -`, - ); - - vi.spyOn(Storage, 'getUserSkillsDir').mockReturnValue(userDir); - const storage = new Storage('/dummy'); - vi.spyOn(storage, 'getProjectSkillsDir').mockReturnValue(projectDir); - - const service = new SkillManager(); - await service.discoverSkills(storage); - - const skills = service.getSkills(); - expect(skills).toHaveLength(1); - expect(skills[0].description).toBe('project-desc'); - }); - - it('should filter disabled skills in getSkills but not in getAllSkills', async () => { - const skill1Dir = path.join(testRootDir, 'skill1'); - const skill2Dir = path.join(testRootDir, 'skill2'); - await fs.mkdir(skill1Dir, { recursive: true }); - await fs.mkdir(skill2Dir, { recursive: true }); - - await fs.writeFile( - path.join(skill1Dir, 'SKILL.md'), - `--- -name: skill1 -description: desc1 ---- -`, - ); - await fs.writeFile( - path.join(skill2Dir, 'SKILL.md'), - `--- -name: skill2 -description: desc2 ---- -`, - ); - - const service = new SkillManager(); - const discovered = await service.discoverSkillsInternal([testRootDir]); - service['addSkillsWithPrecedence'](discovered); - service.setDisabledSkills(['skill1']); - - expect(service.getSkills()).toHaveLength(1); - expect(service.getSkills()[0].name).toBe('skill2'); - expect(service.getAllSkills()).toHaveLength(2); - expect( - service.getAllSkills().find((s) => s.name === 'skill1')?.disabled, - ).toBe(true); - }); -}); diff --git a/packages/core/src/services/skillManager.ts b/packages/core/src/services/skillManager.ts deleted file mode 100644 index 846f705548..0000000000 --- a/packages/core/src/services/skillManager.ts +++ /dev/null @@ -1,197 +0,0 @@ -/** - * @license - * Copyright 2025 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import * as fs from 'node:fs/promises'; -import * as path from 'node:path'; -import { glob } from 'glob'; -import yaml from 'js-yaml'; -import { debugLogger } from '../utils/debugLogger.js'; -import { Storage } from '../config/storage.js'; - -export interface SkillMetadata { - name: string; - description: string; - location: string; - body: string; - disabled?: boolean; -} - -const FRONTMATTER_REGEX = /^---\r?\n([\s\S]*?)\r?\n---\r?\n([\s\S]*)/; - -export class SkillManager { - private skills: SkillMetadata[] = []; - private activeSkillNames: Set = new Set(); - - /** - * Clears all discovered skills. - */ - clearSkills(): void { - this.skills = []; - } - - /** - * Discovers skills from standard user and project locations. - * Project skills take precedence over user skills. - */ - async discoverSkills(storage: Storage): Promise { - this.clearSkills(); - - // User skills first - const userPaths = [Storage.getUserSkillsDir()]; - const userSkills = await this.discoverSkillsInternal(userPaths); - this.addSkillsWithPrecedence(userSkills); - - // Project skills second (overwrites user skills with same name) - const projectPaths = [storage.getProjectSkillsDir()]; - const projectSkills = await this.discoverSkillsInternal(projectPaths); - this.addSkillsWithPrecedence(projectSkills); - } - - private addSkillsWithPrecedence(newSkills: SkillMetadata[]): void { - const skillMap = new Map(); - for (const skill of [...this.skills, ...newSkills]) { - skillMap.set(skill.name, skill); - } - this.skills = Array.from(skillMap.values()); - } - - /** - * Discovered skills in the provided paths and adds them to the manager. - * Internal helper for tiered discovery. - */ - async discoverSkillsInternal(paths: string[]): Promise { - const discoveredSkills: SkillMetadata[] = []; - const seenLocations = new Set(this.skills.map((s) => s.location)); - - for (const searchPath of paths) { - try { - const absoluteSearchPath = path.resolve(searchPath); - debugLogger.debug(`Discovering skills in: ${absoluteSearchPath}`); - - const stats = await fs.stat(absoluteSearchPath).catch(() => null); - if (!stats || !stats.isDirectory()) { - debugLogger.debug( - `Search path is not a directory: ${absoluteSearchPath}`, - ); - continue; - } - - const skillFiles = await glob('*/SKILL.md', { - cwd: absoluteSearchPath, - absolute: true, - nodir: true, - }); - - debugLogger.debug( - `Found ${skillFiles.length} potential skill files in ${absoluteSearchPath}`, - ); - - for (const skillFile of skillFiles) { - if (seenLocations.has(skillFile)) { - continue; - } - - const metadata = await this.parseSkillFile(skillFile); - if (metadata) { - debugLogger.debug( - `Discovered skill: ${metadata.name} at ${skillFile}`, - ); - discoveredSkills.push(metadata); - seenLocations.add(skillFile); - } - } - } catch (error) { - debugLogger.log(`Error discovering skills in ${searchPath}:`, error); - } - } - - return discoveredSkills; - } - - /** - * Returns the list of enabled discovered skills. - */ - getSkills(): SkillMetadata[] { - return this.skills.filter((s) => !s.disabled); - } - - /** - * Returns all discovered skills, including disabled ones. - */ - getAllSkills(): SkillMetadata[] { - return this.skills; - } - - /** - * Filters discovered skills by name. - */ - filterSkills(predicate: (skill: SkillMetadata) => boolean): void { - this.skills = this.skills.filter(predicate); - } - - /** - * Sets the list of disabled skill names. - */ - setDisabledSkills(disabledNames: string[]): void { - for (const skill of this.skills) { - skill.disabled = disabledNames.includes(skill.name); - } - } - - /** - * Reads the full content (metadata + body) of a skill by name. - */ - getSkill(name: string): SkillMetadata | null { - return this.skills.find((s) => s.name === name) ?? null; - } - - /** - * Activates a skill by name. - */ - activateSkill(name: string): void { - this.activeSkillNames.add(name); - } - - /** - * Checks if a skill is active. - */ - isSkillActive(name: string): boolean { - return this.activeSkillNames.has(name); - } - - private async parseSkillFile( - filePath: string, - ): Promise { - try { - const content = await fs.readFile(filePath, 'utf-8'); - const match = content.match(FRONTMATTER_REGEX); - if (!match) { - return null; - } - - // Use yaml.load() which is safe in js-yaml v4. - const frontmatter = yaml.load(match[1]); - if (!frontmatter || typeof frontmatter !== 'object') { - return null; - } - - const { name, description } = frontmatter as Record; - if (typeof name !== 'string' || typeof description !== 'string') { - return null; - } - - return { - name, - description, - location: filePath, - body: match[2].trim(), - }; - } catch (error) { - debugLogger.log(`Error parsing skill file ${filePath}:`, error); - return null; - } - } -} diff --git a/packages/core/src/skills/skillLoader.test.ts b/packages/core/src/skills/skillLoader.test.ts new file mode 100644 index 0000000000..5988b853b4 --- /dev/null +++ b/packages/core/src/skills/skillLoader.test.ts @@ -0,0 +1,69 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { loadSkillsFromDir } from './skillLoader.js'; + +describe('skillLoader', () => { + let testRootDir: string; + + beforeEach(async () => { + testRootDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'skill-loader-test-'), + ); + }); + + afterEach(async () => { + await fs.rm(testRootDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + it('should load skills from a directory with valid SKILL.md', async () => { + const skillDir = path.join(testRootDir, 'my-skill'); + await fs.mkdir(skillDir, { recursive: true }); + const skillFile = path.join(skillDir, 'SKILL.md'); + await fs.writeFile( + skillFile, + `---\nname: my-skill\ndescription: A test skill\n---\n# Instructions\nDo something.\n`, + ); + + const skills = await loadSkillsFromDir(testRootDir); + + expect(skills).toHaveLength(1); + expect(skills[0].name).toBe('my-skill'); + expect(skills[0].description).toBe('A test skill'); + expect(skills[0].location).toBe(skillFile); + expect(skills[0].body).toBe('# Instructions\nDo something.'); + }); + + it('should ignore directories without SKILL.md', async () => { + const notASkillDir = path.join(testRootDir, 'not-a-skill'); + await fs.mkdir(notASkillDir, { recursive: true }); + + const skills = await loadSkillsFromDir(testRootDir); + + expect(skills).toHaveLength(0); + }); + + it('should ignore SKILL.md without valid frontmatter', async () => { + const skillDir = path.join(testRootDir, 'invalid-skill'); + await fs.mkdir(skillDir, { recursive: true }); + const skillFile = path.join(skillDir, 'SKILL.md'); + await fs.writeFile(skillFile, '# No frontmatter here'); + + const skills = await loadSkillsFromDir(testRootDir); + + expect(skills).toHaveLength(0); + }); + + it('should return empty array for non-existent directory', async () => { + const skills = await loadSkillsFromDir('/non/existent/path'); + expect(skills).toEqual([]); + }); +}); diff --git a/packages/core/src/skills/skillLoader.ts b/packages/core/src/skills/skillLoader.ts new file mode 100644 index 0000000000..9dc86fd170 --- /dev/null +++ b/packages/core/src/skills/skillLoader.ts @@ -0,0 +1,98 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; +import { glob } from 'glob'; +import yaml from 'js-yaml'; +import { debugLogger } from '../utils/debugLogger.js'; + +/** + * Represents the definition of an Agent Skill. + */ +export interface SkillDefinition { + /** The unique name of the skill. */ + name: string; + /** A concise description of what the skill does. */ + description: string; + /** The absolute path to the skill's source file on disk. */ + location: string; + /** The core logic/instructions of the skill. */ + body: string; + /** Whether the skill is currently disabled. */ + disabled?: boolean; +} + +const FRONTMATTER_REGEX = /^---\r?\n([\s\S]*?)\r?\n---\r?\n([\s\S]*)/; + +/** + * Discovers and loads all skills in the provided directory. + */ +export async function loadSkillsFromDir( + dir: string, +): Promise { + const discoveredSkills: SkillDefinition[] = []; + + try { + const absoluteSearchPath = path.resolve(dir); + const stats = await fs.stat(absoluteSearchPath).catch(() => null); + if (!stats || !stats.isDirectory()) { + return []; + } + + const skillFiles = await glob('*/SKILL.md', { + cwd: absoluteSearchPath, + absolute: true, + nodir: true, + }); + + for (const skillFile of skillFiles) { + const metadata = await loadSkillFromFile(skillFile); + if (metadata) { + discoveredSkills.push(metadata); + } + } + } catch (error) { + debugLogger.log(`Error discovering skills in ${dir}:`, error); + } + + return discoveredSkills; +} + +/** + * Loads a single skill from a SKILL.md file. + */ +export async function loadSkillFromFile( + filePath: string, +): Promise { + try { + const content = await fs.readFile(filePath, 'utf-8'); + const match = content.match(FRONTMATTER_REGEX); + if (!match) { + return null; + } + + const frontmatter = yaml.load(match[1]); + if (!frontmatter || typeof frontmatter !== 'object') { + return null; + } + + const { name, description } = frontmatter as Record; + if (typeof name !== 'string' || typeof description !== 'string') { + return null; + } + + return { + name, + description, + location: filePath, + body: match[2].trim(), + }; + } catch (error) { + debugLogger.log(`Error parsing skill file ${filePath}:`, error); + return null; + } +} diff --git a/packages/core/src/skills/skillManager.test.ts b/packages/core/src/skills/skillManager.test.ts new file mode 100644 index 0000000000..47e2ae39b2 --- /dev/null +++ b/packages/core/src/skills/skillManager.test.ts @@ -0,0 +1,88 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { SkillManager } from './skillManager.js'; +import { Storage } from '../config/storage.js'; + +describe('SkillManager', () => { + let testRootDir: string; + + beforeEach(async () => { + testRootDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'skill-manager-test-'), + ); + }); + + afterEach(async () => { + await fs.rm(testRootDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + it('should discover skills from Storage with project precedence', async () => { + const userDir = path.join(testRootDir, 'user'); + const projectDir = path.join(testRootDir, 'project'); + await fs.mkdir(path.join(userDir, 'skill-a'), { recursive: true }); + await fs.mkdir(path.join(projectDir, 'skill-a'), { recursive: true }); + + await fs.writeFile( + path.join(userDir, 'skill-a', 'SKILL.md'), + `--- +name: skill-a +description: user-desc +--- +`, + ); + await fs.writeFile( + path.join(projectDir, 'skill-a', 'SKILL.md'), + `--- +name: skill-a +description: project-desc +--- +`, + ); + + vi.spyOn(Storage, 'getUserSkillsDir').mockReturnValue(userDir); + const storage = new Storage('/dummy'); + vi.spyOn(storage, 'getProjectSkillsDir').mockReturnValue(projectDir); + + const service = new SkillManager(); + await service.discoverSkills(storage); + + const skills = service.getSkills(); + expect(skills).toHaveLength(1); + expect(skills[0].description).toBe('project-desc'); + }); + + it('should filter disabled skills in getSkills but not in getAllSkills', async () => { + const skillDir = path.join(testRootDir, 'skill1'); + await fs.mkdir(skillDir, { recursive: true }); + + await fs.writeFile( + path.join(skillDir, 'SKILL.md'), + `--- +name: skill1 +description: desc1 +--- +`, + ); + + const storage = new Storage('/dummy'); + vi.spyOn(storage, 'getProjectSkillsDir').mockReturnValue(testRootDir); + vi.spyOn(Storage, 'getUserSkillsDir').mockReturnValue('/non-existent'); + + const service = new SkillManager(); + await service.discoverSkills(storage); + service.setDisabledSkills(['skill1']); + + expect(service.getSkills()).toHaveLength(0); + expect(service.getAllSkills()).toHaveLength(1); + expect(service.getAllSkills()[0].disabled).toBe(true); + }); +}); diff --git a/packages/core/src/skills/skillManager.ts b/packages/core/src/skills/skillManager.ts new file mode 100644 index 0000000000..a8e14ba388 --- /dev/null +++ b/packages/core/src/skills/skillManager.ts @@ -0,0 +1,97 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { Storage } from '../config/storage.js'; +import { type SkillDefinition, loadSkillsFromDir } from './skillLoader.js'; + +export class SkillManager { + private skills: SkillDefinition[] = []; + private activeSkillNames: Set = new Set(); + + /** + * Clears all discovered skills. + */ + clearSkills(): void { + this.skills = []; + } + + /** + * Discovers skills from standard user and project locations. + * Project skills take precedence over user skills. + */ + async discoverSkills(storage: Storage): Promise { + this.clearSkills(); + + // 1. User skills + const userSkills = await loadSkillsFromDir(Storage.getUserSkillsDir()); + this.addSkillsWithPrecedence(userSkills); + + // 2. Project skills (highest precedence) + const projectSkills = await loadSkillsFromDir( + storage.getProjectSkillsDir(), + ); + this.addSkillsWithPrecedence(projectSkills); + } + + private addSkillsWithPrecedence(newSkills: SkillDefinition[]): void { + const skillMap = new Map(); + for (const skill of [...this.skills, ...newSkills]) { + skillMap.set(skill.name, skill); + } + this.skills = Array.from(skillMap.values()); + } + + /** + * Returns the list of enabled discovered skills. + */ + getSkills(): SkillDefinition[] { + return this.skills.filter((s) => !s.disabled); + } + + /** + * Returns all discovered skills, including disabled ones. + */ + getAllSkills(): SkillDefinition[] { + return this.skills; + } + + /** + * Filters discovered skills by name. + */ + filterSkills(predicate: (skill: SkillDefinition) => boolean): void { + this.skills = this.skills.filter(predicate); + } + + /** + * Sets the list of disabled skill names. + */ + setDisabledSkills(disabledNames: string[]): void { + for (const skill of this.skills) { + skill.disabled = disabledNames.includes(skill.name); + } + } + + /** + * Reads the full content (metadata + body) of a skill by name. + */ + getSkill(name: string): SkillDefinition | null { + return this.skills.find((s) => s.name === name) ?? null; + } + + /** + * Activates a skill by name. + */ + activateSkill(name: string): void { + this.activeSkillNames.add(name); + } + + /** + * Checks if a skill is active. + */ + isSkillActive(name: string): boolean { + return this.activeSkillNames.has(name); + } +}