feat: update skill-tool to OpenCode v1.1.50 functionality (#50)

* feat(skill-tool): update description format to match OpenCode v1.1.50

* feat(skill-tool): update execute output format with skill_content wrapper and file discovery

* test(skill-tool): update tests for v1.1.50 output format

* fix(tests): use optional chaining instead of non-null assertion

* fix(skill-tool): sort discovered files for deterministic output

* fix(skill-tool): remove duplicate closing brace

* test(skill-tool): update assertions for absolute paths and hidden files

* refactor(skill-tool): use walkDir utility for file discovery

* fix(skill-tool): match OpenCode file discovery and omit empty skill_files

Author: marcusrbrown

src/lib/skill-tool.ts

@@ -1,4 +1,6 @@
+import fs from 'node:fs'
 import path from 'node:path'
+import { pathToFileURL } from 'node:url'
 import type { ToolDefinition } from '@opencode-ai/plugin'
 import { tool } from '@opencode-ai/plugin/tool'
 import {
@@ -26,12 +28,62 @@ export function formatSkillsXml(skills: SkillInfo[]): string {
     '  <skill>',
     `    <name>systematic:${skill.name}</name>`,
     `    <description>${skill.description}</description>`,
+    `    <location>${pathToFileURL(skill.path).href}</location>`,
     '  </skill>',
   ])
 
   return ['<available_skills>', ...skillLines, '</available_skills>'].join(' ')
 }
 
+/**
+ * Discovers skill files in a directory and formats them as XML tags.
+ * Recursively searches subdirectories, includes hidden files, excludes .git and SKILL.md.
+ * Matches OpenCode v1.1.50 behavior exactly.
+ *
+ * @param dir - Directory path to search for skill files
+ * @param limit - Maximum number of files to return (default: 10)
+ * @returns String with absolute file paths formatted as XML tags, one per line
+ */
+export function discoverSkillFiles(dir: string, limit = 10): string {
+  const files: string[] = []
+
+  function shouldSkipDirectory(name: string): boolean {
+    return name === '.git'
+  }
+
+  function shouldIncludeFile(name: string): boolean {
+    return name !== 'SKILL.md'
+  }
+
+  function handleEntry(entry: fs.Dirent, currentDir: string): void {
+    if (entry.isDirectory()) {
+      if (!shouldSkipDirectory(entry.name)) {
+        recurse(path.resolve(currentDir, entry.name))
+      }
+    } else if (shouldIncludeFile(entry.name)) {
+      files.push(path.resolve(currentDir, entry.name))
+    }
+  }
+
+  function recurse(currentDir: string): void {
+    if (files.length >= limit) return
+
+    try {
+      const entries = fs.readdirSync(currentDir, { withFileTypes: true })
+
+      for (const entry of entries) {
+        if (files.length >= limit) break
+        handleEntry(entry, currentDir)
+      }
+    } catch {
+      // Silently ignore read errors
+    }
+  }
+
+  recurse(dir)
+  return files.map((file) => `  <file>${file}</file>`).join('\n')
+}
+
 export function createSkillTool(options: SkillToolOptions): ToolDefinition {
   const { bundledSkillsDir, disabledSkills } = options
 
@@ -60,12 +112,19 @@ export function createSkillTool(options: SkillToolOptions): ToolDefinition {
     const systematicXml = formatSkillsXml(skillInfos)
 
     return [
-      'Load a skill to get detailed instructions for a specific task.',
-      'Skills provide specialized knowledge and step-by-step guidance.',
-      "Use this when a task matches an available skill's description.",
-      'Only the skills listed here are available:',
+      'Load a specialized skill that provides domain-specific instructions and workflows.',
+      '',
+      'When you recognize that a task matches one of the available skills listed below, use this tool to load the full skill instructions.',
+      '',
+      'The skill will inject detailed instructions, workflows, and access to bundled resources (scripts, references, templates) into the conversation context.',
+      '',
+      'Tool output includes a `<skill_content name="...">` block with the loaded content.',
+      '',
+      'The following skills provide specialized sets of instructions for particular tasks.',
+      'Invoke this tool to load a skill when a task matches one of the available skills listed below:',
+      '',
       systematicXml,
-    ].join(' ')
+    ].join('\n')
   }
 
   const buildParameterHint = (): string => {
@@ -75,7 +134,7 @@ export function createSkillTool(options: SkillToolOptions): ToolDefinition {
       .map((s) => `'systematic:${s.name}'`)
       .join(', ')
     const hint = examples.length > 0 ? ` (e.g., ${examples}, ...)` : ''
-    return `The skill identifier from available_skills${hint}`
+    return `The name of the skill from available_skills${hint}`
   }
 
   let cachedDescription: string | null = null
@@ -117,6 +176,8 @@ export function createSkillTool(options: SkillToolOptions): ToolDefinition {
 
       const body = extractSkillBody(matchedSkill.wrappedTemplate)
       const dir = path.dirname(matchedSkill.skillFile)
+      const base = pathToFileURL(dir).href
+      const files = discoverSkillFiles(dir)
 
       await context.ask({
         permission: 'skill',
@@ -133,13 +194,23 @@ export function createSkillTool(options: SkillToolOptions): ToolDefinition {
         },
       })
 
-      return [
-        `## Skill: ${matchedSkill.prefixedName}`,
-        '',
-        `**Base directory**: ${dir}`,
+      const output = [
+        `<skill_content name="${matchedSkill.prefixedName}">`,
+        `# Skill: ${matchedSkill.prefixedName}`,
         '',
         body.trim(),
-      ].join('\n')
+        '',
+        `Base directory for this skill: ${base}`,
+        'Relative paths in this skill (e.g., scripts/, reference/) are relative to this base directory.',
+        'Note: file list is sampled.',
+      ]
+
+      if (files) {
+        output.push('', '<skill_files>', files, '</skill_files>')
+      }
+
+      output.push('</skill_content>')
+      return output.join('\n')
     },
   })
 }

tests/unit/skill-tool.test.ts

@@ -26,7 +26,7 @@ describe('skill-tool', () => {
       expect(result).toBe('')
     })
 
-    test('formats single skill with space delimiters and indented structure', () => {
+    test('formats single skill with space delimiters, indented structure, and location field', () => {
       const result = formatSkillsXml([
         {
           path: '/test/path',
@@ -35,9 +35,13 @@ describe('skill-tool', () => {
           description: 'A test skill',
         },
       ])
-      expect(result).toBe(
-        '<available_skills>   <skill>     <name>systematic:test-skill</name>     <description>A test skill</description>   </skill> </available_skills>',
-      )
+      expect(result).toContain('<available_skills>')
+      expect(result).toContain('</available_skills>')
+      expect(result).toContain('<name>systematic:test-skill</name>')
+      expect(result).toContain('<description>A test skill</description>')
+      expect(result).toContain('<location>file:///test/path</location>')
+      // Ensure space-delimited format (no newlines)
+      expect(result).not.toContain('\n')
     })
 
     test('formats multiple skills with space delimiters and indented structure', () => {
@@ -235,7 +239,7 @@ No frontmatter visible here.`,
       expect(result).toContain('# Actual Content')
     })
 
-    test('extracts body from wrapped template (matches OMO pattern)', async () => {
+    test('wraps output with skill_content tags and omits skill_files when no files found', async () => {
       const skillDir = path.join(testDir, 'wrap-test')
       fs.mkdirSync(skillDir)
       fs.writeFileSync(
@@ -254,11 +258,98 @@ description: Test wrapper
 
       const result = await tool.execute({ name: 'wrap-test' }, mockContext)
 
-      expect(result).toContain('## Skill: systematic:wrap-test')
-      expect(result).toContain('**Base directory**:')
+      // New wrapper format
+      expect(result).toContain('<skill_content name="systematic:wrap-test">')
+      expect(result).toContain('</skill_content>')
+      // New heading format
+      expect(result).toContain('# Skill: systematic:wrap-test')
+      // New base directory format with file:// URL
+      expect(result).toContain('Base directory for this skill: file://')
       expect(result).toContain('# Wrapped Content')
-      expect(result).not.toContain('<skill-instruction>')
-      expect(result).not.toContain('</skill-instruction>')
+      // skill_files section should be omitted when no files
+      expect(result).not.toContain('<skill_files>')
+      expect(result).not.toContain('</skill_files>')
+    })
+
+    test('includes discovered files in skill_files section', async () => {
+      const skillDir = path.join(testDir, 'file-discovery-test')
+      fs.mkdirSync(skillDir)
+      fs.writeFileSync(
+        path.join(skillDir, 'SKILL.md'),
+        `---
+name: file-discovery-test
+description: Test file discovery
+---
+# Test Content`,
+      )
+      // Add extra files to be discovered
+      fs.writeFileSync(
+        path.join(skillDir, 'helper.ts'),
+        'export function helper() {}',
+      )
+      fs.writeFileSync(
+        path.join(skillDir, 'utils.ts'),
+        'export function util() {}',
+      )
+      fs.writeFileSync(path.join(skillDir, '.hidden'), 'hidden file')
+
+      const tool = createSkillTool({
+        bundledSkillsDir: testDir,
+        disabledSkills: [],
+      })
+
+      const result = await tool.execute(
+        { name: 'file-discovery-test' },
+        mockContext,
+      )
+
+      expect(result).toContain('<skill_files>')
+      expect(result).toContain('</skill_files>')
+      // Check for absolute paths ending with the filenames
+      expect(result).toMatch(/<file>.*\/helper\.ts<\/file>/)
+      expect(result).toMatch(/<file>.*\/utils\.ts<\/file>/)
+      // SKILL.md should not be in the file list
+      expect(result).not.toContain('<file>SKILL.md</file>')
+      // Hidden files should be included (matches OpenCode v1.1.50 behavior)
+      expect(result).toMatch(/<file>.*\/\.hidden<\/file>/)
+    })
+
+    test('enforces 10-file limit in skill_files section', async () => {
+      const skillDir = path.join(testDir, 'file-limit-test')
+      fs.mkdirSync(skillDir)
+      fs.writeFileSync(
+        path.join(skillDir, 'SKILL.md'),
+        `---
+name: file-limit-test
+description: Test file limit
+---
+# Test Content`,
+      )
+      // Create 15 extra files
+      for (let i = 1; i <= 15; i++) {
+        fs.writeFileSync(
+          path.join(skillDir, `file${i}.ts`),
+          `export const file${i} = ${i}`,
+        )
+      }
+
+      const tool = createSkillTool({
+        bundledSkillsDir: testDir,
+        disabledSkills: [],
+      })
+
+      const result = await tool.execute(
+        { name: 'file-limit-test' },
+        mockContext,
+      )
+
+      // Count the number of <file> tags
+      const fileMatches = result.match(/<file>/g)
+      expect(fileMatches).toBeDefined()
+      expect(fileMatches?.length).toBe(10)
+      // Verify at least one of the first 10 files is present
+      const hasLimitedFiles = /file[0-9]\.ts/.test(result)
+      expect(hasLimitedFiles).toBe(true)
     })
   })
 })