fix: allow disable-model-invocation skills to be loaded via systematic_skill tool (#80)

Skills with disable-model-invocation: true were filtered from both the tool
description and the execute path, making them completely unreachable. Split
getSystematicSkills() into getAllSkills() (for execution) and
getDiscoverableSkills() (for description/hints) so hidden skills can still
be loaded by explicit name while remaining absent from AI-visible context.

Author: marcusrbrown

src/lib/skill-tool.ts

@@ -87,17 +87,20 @@ export function discoverSkillFiles(dir: string, limit = 10): string {
 export function createSkillTool(options: SkillToolOptions): ToolDefinition {
   const { bundledSkillsDir, disabledSkills } = options
 
-  const getSystematicSkills = (): LoadedSkill[] => {
+  const getAllSkills = (): LoadedSkill[] => {
     return findSkillsInDir(bundledSkillsDir)
       .filter((s) => !disabledSkills.includes(s.name))
       .map((skillInfo) => loadSkill(skillInfo))
       .filter((s): s is LoadedSkill => s !== null)
-      .filter((s) => s.disableModelInvocation !== true)
       .sort((a, b) => a.name.localeCompare(b.name))
   }
 
+  const getDiscoverableSkills = (): LoadedSkill[] => {
+    return getAllSkills().filter((s) => s.disableModelInvocation !== true)
+  }
+
   const buildDescription = (): string => {
-    const skills = getSystematicSkills()
+    const skills = getDiscoverableSkills()
 
     if (skills.length === 0) {
       return 'Load a skill to get detailed instructions for a specific task. No skills are currently available.'
@@ -128,7 +131,7 @@ export function createSkillTool(options: SkillToolOptions): ToolDefinition {
   }
 
   const buildParameterHint = (): string => {
-    const skills = getSystematicSkills()
+    const skills = getDiscoverableSkills()
     const examples = skills
       .slice(0, 3)
       .map((s) => `'systematic:${s.name}'`)
@@ -164,11 +167,13 @@ export function createSkillTool(options: SkillToolOptions): ToolDefinition {
         ? requestedName.slice('systematic:'.length)
         : requestedName
 
-      const skills = getSystematicSkills()
+      const skills = getAllSkills()
       const matchedSkill = skills.find((s) => s.name === normalizedName)
 
       if (!matchedSkill) {
-        const availableSystematic = skills.map((s) => s.prefixedName)
+        const availableSystematic = getDiscoverableSkills().map(
+          (s) => s.prefixedName,
+        )
         throw new Error(
           `Skill "${requestedName}" not found. Available systematic skills: ${availableSystematic.join(', ')}`,
         )

tests/unit/skill-tool.test.ts

@@ -144,6 +144,40 @@ description: Disabled
       expect(tool.description).toContain('systematic:enabled-skill')
       expect(tool.description).not.toContain('systematic:disabled-skill')
     })
+
+    test('excludes disableModelInvocation skills from description', () => {
+      const visibleDir = path.join(testDir, 'visible-skill')
+      const hiddenDir = path.join(testDir, 'hidden-skill')
+      fs.mkdirSync(visibleDir)
+      fs.mkdirSync(hiddenDir)
+
+      fs.writeFileSync(
+        path.join(visibleDir, 'SKILL.md'),
+        `---
+name: visible-skill
+description: Visible to model
+---
+# Content`,
+      )
+
+      fs.writeFileSync(
+        path.join(hiddenDir, 'SKILL.md'),
+        `---
+name: hidden-skill
+description: Hidden from model
+disable-model-invocation: true
+---
+# Content`,
+      )
+
+      const tool = createSkillTool({
+        bundledSkillsDir: testDir,
+        disabledSkills: [],
+      })
+
+      expect(tool.description).toContain('systematic:visible-skill')
+      expect(tool.description).not.toContain('systematic:hidden-skill')
+    })
   })
 
   describe('execute', () => {
@@ -351,5 +385,79 @@ description: Test file limit
       const hasLimitedFiles = /file[0-9]\.ts/.test(result)
       expect(hasLimitedFiles).toBe(true)
     })
+
+    test('loads disableModelInvocation skill when explicitly requested', async () => {
+      const hiddenDir = path.join(testDir, 'hidden-skill')
+      fs.mkdirSync(hiddenDir)
+      fs.writeFileSync(
+        path.join(hiddenDir, 'SKILL.md'),
+        `---
+name: hidden-skill
+description: Hidden from model
+disable-model-invocation: true
+---
+# Hidden Skill Content
+
+This skill is only loadable by explicit request.`,
+      )
+
+      const tool = createSkillTool({
+        bundledSkillsDir: testDir,
+        disabledSkills: [],
+      })
+
+      expect(tool.description).not.toContain('hidden-skill')
+
+      const result = await tool.execute(
+        { name: 'systematic:hidden-skill' },
+        mockContext,
+      )
+
+      expect(result).toContain('systematic:hidden-skill')
+      expect(result).toContain('# Hidden Skill Content')
+      expect(result).toContain(
+        'This skill is only loadable by explicit request.',
+      )
+    })
+
+    test('does not list disableModelInvocation skills in error suggestions', async () => {
+      const visibleDir = path.join(testDir, 'visible-skill')
+      const hiddenDir = path.join(testDir, 'hidden-skill')
+      fs.mkdirSync(visibleDir)
+      fs.mkdirSync(hiddenDir)
+
+      fs.writeFileSync(
+        path.join(visibleDir, 'SKILL.md'),
+        `---
+name: visible-skill
+description: Visible to model
+---
+# Content`,
+      )
+
+      fs.writeFileSync(
+        path.join(hiddenDir, 'SKILL.md'),
+        `---
+name: hidden-skill
+description: Hidden from model
+disable-model-invocation: true
+---
+# Content`,
+      )
+
+      const tool = createSkillTool({
+        bundledSkillsDir: testDir,
+        disabledSkills: [],
+      })
+
+      try {
+        await tool.execute({ name: 'nonexistent' }, mockContext)
+        expect.unreachable('Should have thrown')
+      } catch (error) {
+        const message = (error as Error).message
+        expect(message).toContain('systematic:visible-skill')
+        expect(message).not.toContain('hidden-skill')
+      }
+    })
   })
 })