diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index 09a6c4ccf..0c1a947e0 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -1713,6 +1713,21 @@ export class CoreToolScheduler { `\n${rulesCtx}\n`, ); } + + // Activate any conditional skills (skills with `paths:` frontmatter) + // whose globs match this file. Newly activated skills appear in the + // SkillTool listing on the next turn; announce them to the model via + // a system-reminder so it knows they became available. + const activatedSkills = this.config + .getSkillManager() + ?.matchAndActivateByPath(filePath); + if (activatedSkills && activatedSkills.length > 0) { + const names = activatedSkills.join(', '); + content = appendAdditionalContext( + content, + `\nThe following skill(s) are now available via the Skill tool based on the file you just accessed: ${names}. Use them if relevant to the task.\n`, + ); + } } const response = convertToFunctionResponse(toolName, callId, content); diff --git a/packages/core/src/skills/index.ts b/packages/core/src/skills/index.ts index 6cb697e52..d4bc2999c 100644 --- a/packages/core/src/skills/index.ts +++ b/packages/core/src/skills/index.ts @@ -33,3 +33,9 @@ export { SkillError } from './types.js'; // Main management class export { SkillManager } from './skill-manager.js'; + +// Path-based conditional skill activation +export { + SkillActivationRegistry, + splitConditionalSkills, +} from './skill-activation.js'; diff --git a/packages/core/src/skills/skill-activation.test.ts b/packages/core/src/skills/skill-activation.test.ts new file mode 100644 index 000000000..121c8b234 --- /dev/null +++ b/packages/core/src/skills/skill-activation.test.ts @@ -0,0 +1,133 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { + SkillActivationRegistry, + splitConditionalSkills, +} from './skill-activation.js'; +import type { SkillConfig } from './types.js'; + +function makeSkill(overrides: Partial): SkillConfig { + return { + name: overrides.name ?? 'test-skill', + description: overrides.description ?? 'desc', + body: overrides.body ?? '', + level: overrides.level ?? 'project', + filePath: overrides.filePath ?? '/proj/.qwen/skills/test/SKILL.md', + ...overrides, + }; +} + +describe('splitConditionalSkills', () => { + it('treats skills without paths as unconditional', () => { + const skills = [makeSkill({ name: 'a' })]; + const { unconditional, conditional } = splitConditionalSkills(skills); + expect(unconditional).toHaveLength(1); + expect(conditional).toHaveLength(0); + }); + + it('treats empty paths array as unconditional', () => { + const skills = [makeSkill({ name: 'a', paths: [] })]; + const { unconditional, conditional } = splitConditionalSkills(skills); + expect(unconditional).toHaveLength(1); + expect(conditional).toHaveLength(0); + }); + + it('classifies skills with non-empty paths as conditional', () => { + const skills = [ + makeSkill({ name: 'a' }), + makeSkill({ name: 'b', paths: ['src/**/*.tsx'] }), + ]; + const { unconditional, conditional } = splitConditionalSkills(skills); + expect(unconditional.map((s) => s.name)).toEqual(['a']); + expect(conditional.map((s) => s.name)).toEqual(['b']); + }); +}); + +describe('SkillActivationRegistry', () => { + const projectRoot = '/project'; + + it('returns empty when no conditional skills are registered', () => { + const reg = new SkillActivationRegistry([], projectRoot); + expect(reg.matchAndConsume('/project/src/App.tsx')).toEqual([]); + expect(reg.totalCount).toBe(0); + }); + + it('activates a conditional skill when a matching path is touched', () => { + const reg = new SkillActivationRegistry( + [makeSkill({ name: 'tsx-helper', paths: ['src/**/*.tsx'] })], + projectRoot, + ); + const newly = reg.matchAndConsume('/project/src/App.tsx'); + expect(newly).toEqual(['tsx-helper']); + expect(reg.isActivated('tsx-helper')).toBe(true); + expect(reg.activatedCount).toBe(1); + }); + + it('does not re-activate an already-active skill on subsequent matches', () => { + const reg = new SkillActivationRegistry( + [makeSkill({ name: 'tsx-helper', paths: ['src/**/*.tsx'] })], + projectRoot, + ); + expect(reg.matchAndConsume('/project/src/A.tsx')).toEqual(['tsx-helper']); + // Second touch of the same pattern returns nothing new. + expect(reg.matchAndConsume('/project/src/B.tsx')).toEqual([]); + expect(reg.activatedCount).toBe(1); + }); + + it('returns empty for paths that do not match any skill', () => { + const reg = new SkillActivationRegistry( + [makeSkill({ name: 'tsx-helper', paths: ['src/**/*.tsx'] })], + projectRoot, + ); + expect(reg.matchAndConsume('/project/lib/utils.py')).toEqual([]); + }); + + it('activates multiple skills whose globs overlap on a single file', () => { + const reg = new SkillActivationRegistry( + [ + makeSkill({ name: 'tsx-helper', paths: ['src/**/*.tsx'] }), + makeSkill({ name: 'app-helper', paths: ['src/App.tsx'] }), + ], + projectRoot, + ); + const newly = reg.matchAndConsume('/project/src/App.tsx'); + expect(newly.sort()).toEqual(['app-helper', 'tsx-helper']); + }); + + it('accepts relative file paths by resolving against the project root', () => { + const reg = new SkillActivationRegistry( + [makeSkill({ name: 'tsx-helper', paths: ['src/**/*.tsx'] })], + projectRoot, + ); + expect(reg.matchAndConsume('src/App.tsx')).toEqual(['tsx-helper']); + }); + + it('ignores paths outside the project root', () => { + const reg = new SkillActivationRegistry( + [makeSkill({ name: 'tsx-helper', paths: ['src/**/*.tsx'] })], + projectRoot, + ); + expect(reg.matchAndConsume('/other/project/src/App.tsx')).toEqual([]); + expect(reg.activatedCount).toBe(0); + }); + + it('supports multiple glob patterns per skill (OR semantics)', () => { + const reg = new SkillActivationRegistry( + [ + makeSkill({ + name: 'multi', + paths: ['src/**/*.tsx', 'test/**/*.ts'], + }), + ], + projectRoot, + ); + // Both patterns should activate the same skill, but only once total. + expect(reg.matchAndConsume('/project/test/foo.ts')).toEqual(['multi']); + expect(reg.matchAndConsume('/project/src/Bar.tsx')).toEqual([]); + }); +}); diff --git a/packages/core/src/skills/skill-activation.ts b/packages/core/src/skills/skill-activation.ts new file mode 100644 index 000000000..4ede23bb9 --- /dev/null +++ b/packages/core/src/skills/skill-activation.ts @@ -0,0 +1,118 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +// Path-based skill activation (turn-level lazy offering). +// +// Skills with a `paths:` frontmatter are "conditional": they stay out of the +// SkillTool listing until a tool call touches a file matching one of their +// glob patterns. This keeps the model's tool description small in large +// monorepos where most skills are irrelevant to the current task. +// +// Mirrors the design of ConditionalRulesRegistry in utils/rulesDiscovery.ts +// but returns skill names (not content), because the activation affects which +// skills are advertised in SkillTool's description rather than injecting text. + +import * as path from 'node:path'; +import picomatch from 'picomatch'; +import type { SkillConfig } from './types.js'; + +interface CompiledSkill { + readonly skill: SkillConfig; + readonly matchers: picomatch.Matcher[]; +} + +/** + * Splits a skill list into unconditional skills (no `paths:`) and conditional + * skills (with non-empty `paths:`). Unconditional skills are always offered to + * the model; conditional skills only appear after activation. + */ +export function splitConditionalSkills(skills: readonly SkillConfig[]): { + unconditional: SkillConfig[]; + conditional: SkillConfig[]; +} { + const unconditional: SkillConfig[] = []; + const conditional: SkillConfig[] = []; + for (const skill of skills) { + if (skill.paths && skill.paths.length > 0) { + conditional.push(skill); + } else { + unconditional.push(skill); + } + } + return { unconditional, conditional }; +} + +/** + * Tracks which conditional skills have been activated during the session by + * matching tool-invocation file paths against each skill's `paths` globs. + * + * Once activated, a skill stays active for the rest of the registry's + * lifetime. A new registry is constructed on every `refreshCache()` so that + * edits to skill files (adding/removing `paths`) take effect; prior + * activations do not carry over across rebuilds (same as + * ConditionalRulesRegistry). + */ +export class SkillActivationRegistry { + private readonly compiled: CompiledSkill[]; + private readonly activated = new Set(); + private readonly projectRoot: string; + + constructor(conditionalSkills: readonly SkillConfig[], projectRoot: string) { + this.projectRoot = projectRoot; + this.compiled = conditionalSkills.map((skill) => ({ + skill, + matchers: (skill.paths ?? []).map((p) => picomatch(p, { dot: false })), + })); + } + + /** + * Activate any conditional skills whose `paths` globs match `filePath`. + * Returns the names of skills newly activated by this call (empty when + * either no skill matched, or every match was already active). + */ + matchAndConsume(filePath: string): string[] { + if (this.compiled.length === 0) return []; + + const absolutePath = path.isAbsolute(filePath) + ? filePath + : path.resolve(this.projectRoot, filePath); + const relativePath = path + .relative(this.projectRoot, absolutePath) + .replace(/\\/g, '/'); + + // Skip files outside the project root — conditional skills are scoped to + // the project, matching ConditionalRulesRegistry's behavior. + if (relativePath === '..' || relativePath.startsWith('../')) { + return []; + } + + const newlyActivated: string[] = []; + for (const { skill, matchers } of this.compiled) { + if (this.activated.has(skill.name)) continue; + if (matchers.some((m) => m(relativePath))) { + this.activated.add(skill.name); + newlyActivated.push(skill.name); + } + } + return newlyActivated; + } + + isActivated(name: string): boolean { + return this.activated.has(name); + } + + getActivatedNames(): ReadonlySet { + return this.activated; + } + + get totalCount(): number { + return this.compiled.length; + } + + get activatedCount(): number { + return this.activated.size; + } +} diff --git a/packages/core/src/skills/skill-load.ts b/packages/core/src/skills/skill-load.ts index 970441588..9317aefb8 100644 --- a/packages/core/src/skills/skill-load.ts +++ b/packages/core/src/skills/skill-load.ts @@ -115,6 +115,20 @@ export function parseSkillContent( // Extract optional model field const model = parseModelField(frontmatter); + // Optional `paths` frontmatter: glob patterns that gate when this skill + // is offered to the model (conditional skill). + const pathsRaw = frontmatter['paths']; + let paths: string[] | undefined; + if (pathsRaw !== undefined) { + if (!Array.isArray(pathsRaw)) { + throw new Error('"paths" must be an array of glob patterns'); + } + const cleaned = pathsRaw + .map((p) => String(p).trim()) + .filter((p) => p.length > 0); + paths = cleaned.length > 0 ? cleaned : undefined; + } + const config: SkillConfig = { name, description, @@ -123,6 +137,7 @@ export function parseSkillContent( filePath, body: body.trim(), level: 'extension', + paths, }; // Validate the parsed configuration diff --git a/packages/core/src/skills/skill-manager.test.ts b/packages/core/src/skills/skill-manager.test.ts index 9dd23ed62..015d17c69 100644 --- a/packages/core/src/skills/skill-manager.test.ts +++ b/packages/core/src/skills/skill-manager.test.ts @@ -82,6 +82,29 @@ describe('SkillManager', () => { allowedTools: ['read_file', 'write_file'], }; } + if (yamlString.includes('paths:')) { + // Branch handles all three paths-related tests by reading the literal + // YAML so the parser-behavior nuance (array vs scalar vs empty) is + // preserved. + const name = yamlString.includes('name: tsx-helper') + ? 'tsx-helper' + : 'test-skill'; + const description = yamlString.includes('React skill') + ? 'React skill' + : 'A test skill'; + let paths: unknown = undefined; + if (yamlString.includes('paths: []')) { + paths = []; + } else if (yamlString.includes('paths: "src/**/*.tsx"')) { + // Invalid (scalar) — surface as string so our validator rejects it. + paths = 'src/**/*.tsx'; + } else if (yamlString.includes('src/**/*.tsx')) { + paths = yamlString.includes('test/**/*.tsx') + ? ['src/**/*.tsx', 'test/**/*.tsx'] + : ['src/**/*.tsx']; + } + return { name, description, paths }; + } if (yamlString.includes('name: skill1')) { return { name: 'skill1', description: 'First skill' }; } @@ -239,6 +262,76 @@ You are a helpful assistant with this skill. expect(config.allowedTools).toEqual(['read_file', 'write_file']); }); + it('should parse content with paths (conditional skill)', () => { + const markdown = `--- +name: tsx-helper +description: React skill +paths: + - "src/**/*.tsx" + - "test/**/*.tsx" +--- + +Body. +`; + const config = manager.parseSkillContent( + markdown, + validSkillConfig.filePath, + 'project', + ); + expect(config.paths).toEqual(['src/**/*.tsx', 'test/**/*.tsx']); + }); + + it('should leave paths undefined when frontmatter omits it', () => { + const markdown = `--- +name: test-skill +description: A test skill +--- + +Body. +`; + const config = manager.parseSkillContent( + markdown, + validSkillConfig.filePath, + 'project', + ); + expect(config.paths).toBeUndefined(); + }); + + it('should treat an empty paths array as undefined (unconditional)', () => { + const markdown = `--- +name: test-skill +description: A test skill +paths: [] +--- + +Body. +`; + const config = manager.parseSkillContent( + markdown, + validSkillConfig.filePath, + 'project', + ); + expect(config.paths).toBeUndefined(); + }); + + it('should throw when paths is not an array', () => { + const markdown = `--- +name: test-skill +description: A test skill +paths: "src/**/*.tsx" +--- + +Body. +`; + expect(() => + manager.parseSkillContent( + markdown, + validSkillConfig.filePath, + 'project', + ), + ).toThrow(/"paths" must be an array/); + }); + it('should determine level from file path', () => { const projectPath = '/test/project/.qwen/skills/test-skill/SKILL.md'; const userPath = '/home/user/.qwen/skills/test-skill/SKILL.md'; @@ -793,6 +886,81 @@ Review content`); }); }); + describe('conditional skill activation', () => { + // Minimal setup: a project dir containing one conditional skill whose + // paths glob matches `src/**/*.tsx`. After refreshCache() loads it, + // matchAndActivateByPath() should activate it and fire listeners. + async function loadConditionalFixture() { + vi.mocked(fs.readdir).mockResolvedValue([ + { + name: 'tsx-helper', + isDirectory: () => true, + isFile: () => false, + isSymbolicLink: () => false, + }, + ] as unknown as Awaited>); + vi.mocked(fs.access).mockResolvedValue(undefined); + vi.mocked(fs.readFile).mockResolvedValue(`--- +name: tsx-helper +description: React skill +paths: + - "src/**/*.tsx" +--- + +Body. +`); + await manager.refreshCache(); + } + + it('keeps conditional skills inactive until a matching path is touched', async () => { + await loadConditionalFixture(); + + const all = await manager.listSkills(); + const tsx = all.find((s) => s.name === 'tsx-helper'); + expect(tsx).toBeDefined(); + expect(manager.isSkillActive(tsx!)).toBe(false); + }); + + it('activates a conditional skill when a matching file path is touched', async () => { + await loadConditionalFixture(); + + const newly = manager.matchAndActivateByPath('/test/project/src/App.tsx'); + expect(newly).toEqual(['tsx-helper']); + expect(manager.getActivatedSkillNames().has('tsx-helper')).toBe(true); + + const all = await manager.listSkills(); + const tsx = all.find((s) => s.name === 'tsx-helper')!; + expect(manager.isSkillActive(tsx)).toBe(true); + }); + + it('does not re-notify listeners on subsequent matches of the same skill', async () => { + await loadConditionalFixture(); + + const listener = vi.fn(); + manager.addChangeListener(listener); + + expect(manager.matchAndActivateByPath('/test/project/src/A.tsx')).toEqual( + ['tsx-helper'], + ); + expect(listener).toHaveBeenCalledTimes(1); + + // Same pattern touched again — skill already active, no new + // notification. + expect(manager.matchAndActivateByPath('/test/project/src/B.tsx')).toEqual( + [], + ); + expect(listener).toHaveBeenCalledTimes(1); + }); + + it('does nothing for paths outside the project root', async () => { + await loadConditionalFixture(); + expect(manager.matchAndActivateByPath('/other/place/foo.tsx')).toEqual( + [], + ); + expect(manager.getActivatedSkillNames().size).toBe(0); + }); + }); + describe('parse errors', () => { it('should track parse errors', async () => { vi.mocked(fs.readdir).mockResolvedValue([ diff --git a/packages/core/src/skills/skill-manager.ts b/packages/core/src/skills/skill-manager.ts index a0aed671c..ebc611617 100644 --- a/packages/core/src/skills/skill-manager.ts +++ b/packages/core/src/skills/skill-manager.ts @@ -22,6 +22,10 @@ import type { import { SkillError, SkillErrorCode, parseModelField } from './types.js'; import type { Config } from '../config/config.js'; import { validateConfig } from './skill-load.js'; +import { + SkillActivationRegistry, + splitConditionalSkills, +} from './skill-activation.js'; import { createDebugLogger } from '../utils/debugLogger.js'; import { normalizeContent } from '../utils/textUtils.js'; import { SKILL_PROVIDER_CONFIG_DIRS } from '../config/storage.js'; @@ -66,6 +70,7 @@ export class SkillManager { private watchStarted = false; private refreshTimer: NodeJS.Timeout | null = null; private readonly bundledSkillsDir: string; + private activationRegistry: SkillActivationRegistry | null = null; constructor(private readonly config: Config) { this.bundledSkillsDir = path.join( @@ -278,18 +283,61 @@ export class SkillManager { ); let totalSkills = 0; + const allSkills: SkillConfig[] = []; for (const [level, levelSkills] of loaded) { skillsCache.set(level, levelSkills); totalSkills += levelSkills.length; + allSkills.push(...levelSkills); } this.skillsCache = skillsCache; + + // Rebuild the activation registry so that newly added/removed `paths:` + // frontmatter takes effect. Prior activations do not carry across reloads. + const { conditional } = splitConditionalSkills(allSkills); + this.activationRegistry = new SkillActivationRegistry( + conditional, + this.config.getProjectRoot(), + ); + debugLogger.info( - `Skills cache refreshed: ${totalSkills} total skills loaded`, + `Skills cache refreshed: ${totalSkills} total skills loaded ` + + `(${conditional.length} conditional)`, ); this.notifyChangeListeners(); } + /** + * Whether the given skill is currently eligible to appear in the SkillTool + * listing. Unconditional skills are always eligible; conditional skills + * become eligible only after a tool invocation touches a file matching + * their `paths:` globs. + */ + isSkillActive(skill: SkillConfig): boolean { + if (!skill.paths || skill.paths.length === 0) return true; + return this.activationRegistry?.isActivated(skill.name) ?? false; + } + + /** + * Activate any conditional skills whose `paths:` globs match `filePath`. + * Returns the names of skills newly activated by this call. When at least + * one skill activates, change listeners are notified so the SkillTool + * description picks up the new entry on the next refresh. + */ + matchAndActivateByPath(filePath: string): string[] { + if (!this.activationRegistry) return []; + const newly = this.activationRegistry.matchAndConsume(filePath); + if (newly.length > 0) { + this.notifyChangeListeners(); + } + return newly; + } + + /** Names of all conditional skills activated so far (read-only snapshot). */ + getActivatedSkillNames(): ReadonlySet { + return this.activationRegistry?.getActivatedNames() ?? new Set(); + } + /** * Starts watching skill directories for changes. */ @@ -468,6 +516,20 @@ export class SkillManager { ? true : undefined; + // Optional `paths` frontmatter: glob patterns that gate when this skill + // is offered to the model (conditional skill). + const pathsRaw = frontmatter['paths']; + let paths: string[] | undefined; + if (pathsRaw !== undefined) { + if (!Array.isArray(pathsRaw)) { + throw new Error('"paths" must be an array of glob patterns'); + } + const cleaned = pathsRaw + .map((p) => String(p).trim()) + .filter((p) => p.length > 0); + paths = cleaned.length > 0 ? cleaned : undefined; + } + const config: SkillConfig = { name, description, @@ -480,6 +542,7 @@ export class SkillManager { body: body.trim(), whenToUse, disableModelInvocation, + paths, }; // Validate the parsed configuration diff --git a/packages/core/src/skills/types.ts b/packages/core/src/skills/types.ts index fe7332466..84637b2ff 100644 --- a/packages/core/src/skills/types.ts +++ b/packages/core/src/skills/types.ts @@ -95,6 +95,16 @@ export interface SkillConfig { * field in SKILL.md. */ disableModelInvocation?: boolean; + + /** + * Optional glob patterns that gate when this skill is offered to the model. + * When present and non-empty, the skill is a "conditional skill": it stays + * out of the SkillTool listing until a tool invocation touches a file path + * matching one of these patterns, at which point the skill is activated for + * the rest of the session. Patterns are resolved relative to the project + * root and matched via picomatch. Parsed from the `paths` frontmatter field. + */ + paths?: string[]; } /** diff --git a/packages/core/src/tools/skill.test.ts b/packages/core/src/tools/skill.test.ts index a6bdfb0d4..74b5547f2 100644 --- a/packages/core/src/tools/skill.test.ts +++ b/packages/core/src/tools/skill.test.ts @@ -97,6 +97,10 @@ describe('SkillTool', () => { }; }), getParseErrors: vi.fn().mockReturnValue(new Map()), + // Default to "all skills active" so existing tests that use + // unconditional skills are unaffected by the conditional-skill gating + // added alongside `paths:` frontmatter. + isSkillActive: vi.fn().mockReturnValue(true), } as unknown as SkillManager; MockedSkillManager.mockImplementation(() => mockSkillManager); @@ -243,6 +247,64 @@ describe('SkillTool', () => { 'Skill "non-existent" not found. No skills are currently available.', ); }); + + it('returns a path-activation error for a registered but not-yet-activated conditional skill', async () => { + const conditionalSkill: SkillConfig = { + name: 'tsx-helper', + description: 'React TSX helper', + level: 'project', + filePath: '/test/project/.qwen/skills/tsx-helper/SKILL.md', + body: 'Body.', + paths: ['src/**/*.tsx'], + }; + vi.mocked(mockSkillManager.listSkills).mockResolvedValue([ + conditionalSkill, + ]); + // Simulate the skill being registered on disk but not yet activated. + vi.mocked(mockSkillManager.isSkillActive).mockImplementation( + (s: SkillConfig) => !s.paths || s.paths.length === 0, + ); + + const gatedTool = new SkillTool(config); + await vi.runAllTimersAsync(); + + const result = gatedTool.validateToolParams({ skill: 'tsx-helper' }); + expect(result).toMatch(/gated by path-based activation/); + expect(result).toMatch(/paths: frontmatter/); + }); + + it('does not allow a pending conditional skill to be invoked via the model-invocable command path', async () => { + // Regression for /review finding: SkillCommandLoader exposes every + // user/project skill as a model-invocable command. Without dropping + // file-based names from modelInvocableCommands, validateToolParams + // would accept a path-gated skill via the command branch and bypass + // the activation contract entirely. + const conditionalSkill: SkillConfig = { + name: 'tsx-helper', + description: 'React TSX helper', + level: 'project', + filePath: '/test/project/.qwen/skills/tsx-helper/SKILL.md', + body: 'Body.', + paths: ['src/**/*.tsx'], + }; + vi.mocked(mockSkillManager.listSkills).mockResolvedValue([ + conditionalSkill, + ]); + vi.mocked(mockSkillManager.isSkillActive).mockImplementation( + (s: SkillConfig) => !s.paths || s.paths.length === 0, + ); + // SkillCommandLoader would surface tsx-helper here even though it is + // a path-gated file-based skill. + vi.mocked(config.getModelInvocableCommandsProvider).mockReturnValue( + () => [{ name: 'tsx-helper', description: 'React TSX helper' }], + ); + + const gatedTool = new SkillTool(config); + await vi.runAllTimersAsync(); + + const result = gatedTool.validateToolParams({ skill: 'tsx-helper' }); + expect(result).toMatch(/gated by path-based activation/); + }); }); describe('refreshSkills', () => { diff --git a/packages/core/src/tools/skill.ts b/packages/core/src/tools/skill.ts index f2a7a0c7f..51cc1017a 100644 --- a/packages/core/src/tools/skill.ts +++ b/packages/core/src/tools/skill.ts @@ -35,6 +35,12 @@ export class SkillTool extends BaseDeclarativeTool { private skillManager: SkillManager; private availableSkills: SkillConfig[] = []; + // Conditional skills (with `paths:`) that exist on disk but have not yet + // been activated by a matching tool invocation. Tracked separately so + // validateToolParams can give a distinct error message when the model + // names one of these: "gated by paths:, access a matching file first" + // instead of the generic "not found". + private pendingConditionalSkillNames: Set = new Set(); private modelInvocableCommands: ReadonlyArray<{ name: string; description: string; @@ -85,22 +91,45 @@ export class SkillTool extends BaseDeclarativeTool { */ async refreshSkills(): Promise { try { - this.availableSkills = (await this.skillManager.listSkills()).filter( - (s) => !s.disableModelInvocation, + // Include a skill in the tool description only when (a) it is not + // hidden from the model (`disable-model-invocation`), and (b) it is + // either unconditional or already activated by a matching file path + // in this session. This keeps the tool description small in large + // monorepos where most conditional skills are not yet relevant. + const allSkills = await this.skillManager.listSkills(); + this.availableSkills = allSkills.filter( + (s) => !s.disableModelInvocation && this.skillManager.isSkillActive(s), ); - // Merge in model-invocable commands from CommandService (injected via Config), - // but exclude any whose names already appear as file-based skills to avoid - // showing the same skill in both and . + // Track still-pending conditional skills so validateToolParams can + // distinguish "not found" from "registered but not yet activated". + this.pendingConditionalSkillNames = new Set( + allSkills + .filter( + (s) => + !s.disableModelInvocation && + s.paths && + s.paths.length > 0 && + !this.skillManager.isSkillActive(s), + ) + .map((s) => s.name), + ); + // Merge in model-invocable commands from CommandService (injected via + // Config), but exclude any whose names appear as ANY file-based skill — + // including pending conditional skills. Using `availableSkills` (active + // only) here would let a path-gated skill leak through the + // listing and bypass validateToolParams's + // pendingConditionalSkillNames check, breaking the activation contract. const provider = this.config.getModelInvocableCommandsProvider(); const allCommands = provider ? provider() : []; - const skillNames = new Set(this.availableSkills.map((s) => s.name)); + const fileBasedSkillNames = new Set(allSkills.map((s) => s.name)); this.modelInvocableCommands = allCommands.filter( - (cmd) => !skillNames.has(cmd.name), + (cmd) => !fileBasedSkillNames.has(cmd.name), ); this.updateDescriptionAndSchema(); } catch (error) { debugLogger.warn('Failed to load skills for Skills tool:', error); this.availableSkills = []; + this.pendingConditionalSkillNames = new Set(); this.modelInvocableCommands = []; this.updateDescriptionAndSchema(); } finally { @@ -208,6 +237,15 @@ ${skillDescriptions} ); if (commandExists) return null; + // Distinct error for a conditional skill (registered via `paths:` + // frontmatter) that has not yet been activated by a matching tool call. + // Without this branch the model can't tell the difference between "no + // such skill exists" and "exists but you need to access a matching file + // to unlock it." + if (this.pendingConditionalSkillNames.has(params.skill)) { + return `Skill "${params.skill}" is gated by path-based activation (paths: frontmatter) and is not yet available. Access a file matching its paths patterns first to activate it.`; + } + const availableNames = [ ...this.availableSkills.map((s) => s.name), ...this.modelInvocableCommands.map((c) => c.name),