From 83496e3c3c7cf3d1099cdfa9fab7b11703748372 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Sat, 28 Mar 2026 06:18:04 -0700 Subject: [PATCH] fix(sdk): merge skills from both .copilot/skills/ and .squad/skills/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #77 — skill discovery only read one directory, ignoring the other. Changes: - skill-source.ts: Replace skillsDir (either/or) with skillsDirs (array of both dirs). listSkills/getSkill/getContent now scan all dirs with .copilot/skills/ winning on name conflicts. - resolver.ts: Replace .find() in readSkills() with a loop that collects from all candidate dirs, dedup by name (first wins). - squad.agent.md: Fix SKILL EXTRACTION, plugin install, and MCP skill reference paths from .squad/skills/ to .copilot/skills/. - Tests: 8 new skill-source tests + 3 new upstream tests covering merge, dedup, priority, empty dir, backward/forward compat. Closes #77 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/agents/squad.agent.md | 8 +- packages/squad-sdk/src/skills/skill-source.ts | 110 ++++++++-------- packages/squad-sdk/src/upstream/resolver.ts | 27 ++-- test/skill-source.test.ts | 120 ++++++++++++++++++ test/upstream.test.ts | 99 +++++++++++++++ 5 files changed, 299 insertions(+), 65 deletions(-) diff --git a/.github/agents/squad.agent.md b/.github/agents/squad.agent.md index 655641842..1aca52e28 100644 --- a/.github/agents/squad.agent.md +++ b/.github/agents/squad.agent.md @@ -499,7 +499,7 @@ The `sql` tool is **CLI-only**. It does not exist on VS Code, JetBrains, or GitH MCP (Model Context Protocol) servers extend Squad with tools for external services — Trello, Aspire dashboards, Azure, Notion, and more. The user configures MCP servers in their environment; Squad discovers and uses them. -> **Full patterns:** Read `.squad/skills/mcp-tool-discovery/SKILL.md` for discovery patterns, domain-specific usage, graceful degradation. Read `.squad/templates/mcp-config.md` for config file locations, sample configs, and authentication notes. +> **Full patterns:** Read `.copilot/skills/mcp-tool-discovery/SKILL.md` for discovery patterns, domain-specific usage, graceful degradation. Read `.squad/templates/mcp-config.md` for config file locations, sample configs, and authentication notes. #### Detection @@ -822,7 +822,7 @@ prompt: | 2. If you made a team-relevant decision, write to: .squad/decisions/inbox/{name}-{brief-slug}.md 3. SKILL EXTRACTION: If you found a reusable pattern, write/update - .squad/skills/{skill-name}/SKILL.md (read templates/skill.md for format). + .copilot/skills/{skill-name}/SKILL.md (read templates/skill.md for format). ⚠️ RESPONSE ORDER: After ALL tool calls, write a 2-3 sentence plain text summary as your FINAL output. No tool calls after this summary. @@ -910,7 +910,7 @@ Ceremonies are structured team meetings where agents align before or after work. If the user says "I need a designer" or "add someone for DevOps": 1. **Allocate a name** from the current assignment's universe (read from `.squad/casting/history.json`). If the universe is exhausted, apply overflow handling (see Casting & Persistent Naming → Overflow Handling). -2. **Check plugin marketplaces.** If `.squad/plugins/marketplaces.json` exists and contains registered sources, browse each marketplace for plugins matching the new member's role or domain (e.g., "azure-cloud-development" for an Azure DevOps role). Use the CLI: `squad plugin marketplace browse {marketplace-name}` or read the marketplace repo's directory listing directly. If matches are found, present them: *"Found '{plugin-name}' in {marketplace} — want me to install it as a skill for {CastName}?"* If the user accepts, copy the plugin content into `.squad/skills/{plugin-name}/SKILL.md` or merge relevant instructions into the agent's charter. If no marketplaces are configured, skip silently. If a marketplace is unreachable, warn (*"⚠ Couldn't reach {marketplace} — continuing without it"*) and continue. +2. **Check plugin marketplaces.** If `.squad/plugins/marketplaces.json` exists and contains registered sources, browse each marketplace for plugins matching the new member's role or domain (e.g., "azure-cloud-development" for an Azure DevOps role). Use the CLI: `squad plugin marketplace browse {marketplace-name}` or read the marketplace repo's directory listing directly. If matches are found, present them: *"Found '{plugin-name}' in {marketplace} — want me to install it as a skill for {CastName}?"* If the user accepts, copy the plugin content into `.copilot/skills/{plugin-name}/SKILL.md` or merge relevant instructions into the agent's charter. If no marketplaces are configured, skip silently. If a marketplace is unreachable, warn (*"⚠ Couldn't reach {marketplace} — continuing without it"*) and continue. 3. Generate a new charter.md + history.md (seeded with project context from team.md), using the cast name. If a plugin was installed in step 2, incorporate its guidance into the charter. 4. **Update `.squad/casting/registry.json`** with the new agent entry. 5. Add to team.md roster. @@ -933,7 +933,7 @@ If the user wants to remove someone: **Core rules (always loaded):** - Check `.squad/plugins/marketplaces.json` during Add Team Member flow (after name allocation, before charter) - Present matching plugins for user approval -- Install: copy to `.squad/skills/{plugin-name}/SKILL.md`, log to history.md +- Install: copy to `.copilot/skills/{plugin-name}/SKILL.md`, log to history.md - Skip silently if no marketplaces configured --- diff --git a/packages/squad-sdk/src/skills/skill-source.ts b/packages/squad-sdk/src/skills/skill-source.ts index 6cfdc196f..0def25da1 100644 --- a/packages/squad-sdk/src/skills/skill-source.ts +++ b/packages/squad-sdk/src/skills/skill-source.ts @@ -38,73 +38,83 @@ export class LocalSkillSource implements SkillSource { this.priority = priority; } - private get skillsDir(): string { + /** Returns all existing skill directories, .copilot/skills/ first (highest priority). */ + private get skillsDirs(): string[] { + const dirs: string[] = []; const copilotDir = path.join(this.basePath, '.copilot', 'skills'); - if (fs.existsSync(copilotDir)) return copilotDir; - // Backward compat: fall back to legacy location - return path.join(this.basePath, '.squad', 'skills'); + const squadDir = path.join(this.basePath, '.squad', 'skills'); + if (fs.existsSync(copilotDir)) dirs.push(copilotDir); + if (fs.existsSync(squadDir)) dirs.push(squadDir); + return dirs; } async listSkills(): Promise { - if (!fs.existsSync(this.skillsDir)) return []; - let entries: fs.Dirent[]; - try { - entries = fs.readdirSync(this.skillsDir, { withFileTypes: true }); - } catch { - return []; + const seen = new Map(); + for (const dir of this.skillsDirs) { + let entries: fs.Dirent[]; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + continue; + } + for (const entry of entries) { + if (!entry.isDirectory()) continue; + if (seen.has(entry.name)) continue; // first dir wins on conflicts + const skillFile = path.join(dir, entry.name, 'SKILL.md'); + if (!fs.existsSync(skillFile)) continue; + try { + const raw = fs.readFileSync(skillFile, 'utf-8'); + const { meta } = parseFrontmatter(raw); + seen.set(entry.name, { + id: entry.name, + name: typeof meta.name === 'string' ? meta.name : entry.name, + domain: typeof meta.domain === 'string' ? meta.domain : 'general', + source: 'local', + }); + } catch { + // skip malformed + } + } } + return Array.from(seen.values()); + } - const manifests: SkillManifest[] = []; - for (const entry of entries) { - if (!entry.isDirectory()) continue; - const skillFile = path.join(this.skillsDir, entry.name, 'SKILL.md'); + async getSkill(id: string): Promise { + for (const dir of this.skillsDirs) { + const skillFile = path.join(dir, id, 'SKILL.md'); if (!fs.existsSync(skillFile)) continue; try { const raw = fs.readFileSync(skillFile, 'utf-8'); - const { meta } = parseFrontmatter(raw); - manifests.push({ - id: entry.name, - name: typeof meta.name === 'string' ? meta.name : entry.name, + const { meta, body } = parseFrontmatter(raw); + if (!body) continue; + return { + id, + name: typeof meta.name === 'string' ? meta.name : id, domain: typeof meta.domain === 'string' ? meta.domain : 'general', - source: 'local', - }); + content: body, + triggers: Array.isArray(meta.triggers) ? meta.triggers : [], + agentRoles: Array.isArray(meta.roles) ? meta.roles : [], + }; } catch { - // skip malformed + continue; } } - return manifests; - } - - async getSkill(id: string): Promise { - const skillFile = path.join(this.skillsDir, id, 'SKILL.md'); - if (!fs.existsSync(skillFile)) return null; - try { - const raw = fs.readFileSync(skillFile, 'utf-8'); - const { meta, body } = parseFrontmatter(raw); - if (!body) return null; - return { - id, - name: typeof meta.name === 'string' ? meta.name : id, - domain: typeof meta.domain === 'string' ? meta.domain : 'general', - content: body, - triggers: Array.isArray(meta.triggers) ? meta.triggers : [], - agentRoles: Array.isArray(meta.roles) ? meta.roles : [], - }; - } catch { - return null; - } + return null; } async getContent(id: string): Promise { - const skillFile = path.join(this.skillsDir, id, 'SKILL.md'); - if (!fs.existsSync(skillFile)) return null; - try { - const raw = fs.readFileSync(skillFile, 'utf-8'); - const { body } = parseFrontmatter(raw); - return body || null; - } catch { - return null; + for (const dir of this.skillsDirs) { + const skillFile = path.join(dir, id, 'SKILL.md'); + if (!fs.existsSync(skillFile)) continue; + try { + const raw = fs.readFileSync(skillFile, 'utf-8'); + const { body } = parseFrontmatter(raw); + if (body) return body; + } catch { + continue; + } } + return null; } } diff --git a/packages/squad-sdk/src/upstream/resolver.ts b/packages/squad-sdk/src/upstream/resolver.ts index a57720f05..06c2d0dc4 100644 --- a/packages/squad-sdk/src/upstream/resolver.ts +++ b/packages/squad-sdk/src/upstream/resolver.ts @@ -59,22 +59,27 @@ function readSkills(squadDir: string): Array<{ name: string; content: string }> { dir: path.join(squadDir, 'skills'), layout: 'nested' as const }, { dir: path.join(projectDir, '.ai-team', 'skills'), layout: 'flat' as const }, ]; - const source = candidateDirs.find(({ dir }) => fs.existsSync(dir)); - if (!source) return []; + const seen = new Set(); const skills: Array<{ name: string; content: string }> = []; - try { - for (const entry of fs.readdirSync(source.dir)) { - const skillFile = source.layout === 'nested' - ? path.join(source.dir, entry, 'SKILL.md') - : path.join(source.dir, entry); - if (fs.existsSync(skillFile)) { + + for (const source of candidateDirs) { + if (!fs.existsSync(source.dir)) continue; + try { + for (const entry of fs.readdirSync(source.dir)) { + const skillFile = source.layout === 'nested' + ? path.join(source.dir, entry, 'SKILL.md') + : path.join(source.dir, entry); const name = source.layout === 'nested' ? entry : path.basename(entry, '.md'); - skills.push({ name, content: fs.readFileSync(skillFile, 'utf8') }); + if (seen.has(name)) continue; // first dir wins on conflicts + if (fs.existsSync(skillFile)) { + skills.push({ name, content: fs.readFileSync(skillFile, 'utf8') }); + seen.add(name); + } } + } catch { + // Graceful degradation — continue to next candidate dir } - } catch { - // Graceful degradation — return what we found } return skills; } diff --git a/test/skill-source.test.ts b/test/skill-source.test.ts index f8d2fb20e..713527ed3 100644 --- a/test/skill-source.test.ts +++ b/test/skill-source.test.ts @@ -157,6 +157,126 @@ describe('LocalSkillSource', () => { expect(skills[0].id).toBe('legacy-skill'); }); + it('should merge skills from both .copilot/skills/ and .squad/skills/', async () => { + // Skill A only in .copilot/skills/ + const copilotSkill = path.join(tempDir, '.copilot', 'skills', 'copilot-only'); + fs.mkdirSync(copilotSkill, { recursive: true }); + fs.writeFileSync(path.join(copilotSkill, 'SKILL.md'), SKILL_MD); + + // Skill B only in .squad/skills/ + const squadSkill = path.join(tempDir, '.squad', 'skills', 'squad-only'); + fs.mkdirSync(squadSkill, { recursive: true }); + fs.writeFileSync(path.join(squadSkill, 'SKILL.md'), SKILL_MD_MINIMAL); + + const source = new LocalSkillSource(tempDir); + const skills = await source.listSkills(); + + expect(skills).toHaveLength(2); + const ids = skills.map(s => s.id).sort(); + expect(ids).toEqual(['copilot-only', 'squad-only']); + }); + + it('should deduplicate by id with .copilot/skills/ winning on conflicts', async () => { + const COPILOT_VERSION = `---\nname: Copilot Version\ndomain: copilot\n---\nCopilot wins.\n`; + const SQUAD_VERSION = `---\nname: Squad Version\ndomain: squad\n---\nSquad loses.\n`; + + // Same skill name in both dirs + const copilotSkill = path.join(tempDir, '.copilot', 'skills', 'conflict-skill'); + fs.mkdirSync(copilotSkill, { recursive: true }); + fs.writeFileSync(path.join(copilotSkill, 'SKILL.md'), COPILOT_VERSION); + + const squadSkill = path.join(tempDir, '.squad', 'skills', 'conflict-skill'); + fs.mkdirSync(squadSkill, { recursive: true }); + fs.writeFileSync(path.join(squadSkill, 'SKILL.md'), SQUAD_VERSION); + + const source = new LocalSkillSource(tempDir); + const skills = await source.listSkills(); + + expect(skills).toHaveLength(1); + expect(skills[0].name).toBe('Copilot Version'); + expect(skills[0].domain).toBe('copilot'); + }); + + it('should handle empty .copilot/skills/ dir without shadowing .squad/skills/', async () => { + // Create empty .copilot/skills/ directory + fs.mkdirSync(path.join(tempDir, '.copilot', 'skills'), { recursive: true }); + + // Skill only in .squad/skills/ + const squadSkill = path.join(tempDir, '.squad', 'skills', 'legacy-skill'); + fs.mkdirSync(squadSkill, { recursive: true }); + fs.writeFileSync(path.join(squadSkill, 'SKILL.md'), SKILL_MD_MINIMAL); + + const source = new LocalSkillSource(tempDir); + const skills = await source.listSkills(); + + expect(skills).toHaveLength(1); + expect(skills[0].id).toBe('legacy-skill'); + }); + + it('should work when only .copilot/skills/ exists', async () => { + const copilotSkill = path.join(tempDir, '.copilot', 'skills', 'copilot-skill'); + fs.mkdirSync(copilotSkill, { recursive: true }); + fs.writeFileSync(path.join(copilotSkill, 'SKILL.md'), SKILL_MD); + + const source = new LocalSkillSource(tempDir); + const skills = await source.listSkills(); + + expect(skills).toHaveLength(1); + expect(skills[0].id).toBe('copilot-skill'); + }); + + it('getSkill should check .copilot/skills/ first then .squad/skills/', async () => { + const COPILOT_CONTENT = `---\nname: Copilot Skill\ndomain: copilot\ntriggers: [copilot]\nroles: [dev]\n---\nCopilot content.\n`; + const SQUAD_CONTENT = `---\nname: Squad Skill\ndomain: squad\ntriggers: [squad]\nroles: [ops]\n---\nSquad content.\n`; + + // Same ID in both dirs + const copilotSkill = path.join(tempDir, '.copilot', 'skills', 'shared'); + fs.mkdirSync(copilotSkill, { recursive: true }); + fs.writeFileSync(path.join(copilotSkill, 'SKILL.md'), COPILOT_CONTENT); + + const squadSkill = path.join(tempDir, '.squad', 'skills', 'shared'); + fs.mkdirSync(squadSkill, { recursive: true }); + fs.writeFileSync(path.join(squadSkill, 'SKILL.md'), SQUAD_CONTENT); + + const source = new LocalSkillSource(tempDir); + const skill = await source.getSkill('shared'); + + expect(skill).not.toBeNull(); + expect(skill!.name).toBe('Copilot Skill'); + expect(skill!.content).toContain('Copilot content'); + }); + + it('getSkill should find skill in .squad/skills/ when not in .copilot/skills/', async () => { + // .copilot/skills/ exists but doesn't have this skill + fs.mkdirSync(path.join(tempDir, '.copilot', 'skills'), { recursive: true }); + + const squadSkill = path.join(tempDir, '.squad', 'skills', 'squad-only'); + fs.mkdirSync(squadSkill, { recursive: true }); + fs.writeFileSync(path.join(squadSkill, 'SKILL.md'), SKILL_MD_MINIMAL); + + const source = new LocalSkillSource(tempDir); + const skill = await source.getSkill('squad-only'); + + expect(skill).not.toBeNull(); + expect(skill!.name).toBe('Docker'); + }); + + it('getContent should check both directories with priority', async () => { + const copilotSkill = path.join(tempDir, '.copilot', 'skills', 'content-test'); + fs.mkdirSync(copilotSkill, { recursive: true }); + fs.writeFileSync(path.join(copilotSkill, 'SKILL.md'), `---\nname: test\n---\nCopilot body.\n`); + + const squadSkill = path.join(tempDir, '.squad', 'skills', 'content-test'); + fs.mkdirSync(squadSkill, { recursive: true }); + fs.writeFileSync(path.join(squadSkill, 'SKILL.md'), `---\nname: test\n---\nSquad body.\n`); + + const source = new LocalSkillSource(tempDir); + const content = await source.getContent('content-test'); + + expect(content).toContain('Copilot body'); + expect(content).not.toContain('Squad body'); + }); + it('should support custom priority', () => { const source = new LocalSkillSource(tempDir, 10); expect(source.priority).toBe(10); diff --git a/test/upstream.test.ts b/test/upstream.test.ts index 83cf95748..8f3946fe7 100644 --- a/test/upstream.test.ts +++ b/test/upstream.test.ts @@ -188,3 +188,102 @@ describe('upstream resolver', () => { expect(buildSessionDisplay(null)).toBe(''); }); }); + +describe('upstream resolver — readSkills merges both directories', () => { + let sourceDir: string; + let childDir: string; + + function setup() { + sourceDir = makeTempDir('squad-upstream-dual-'); + childDir = makeTempDir('squad-upstream-child-'); + } + + function teardown() { + cleanDir(sourceDir); + cleanDir(childDir); + } + + it('merges skills from .copilot/skills/ and .squad/skills/', () => { + setup(); + try { + const squadDir = path.join(sourceDir, '.squad'); + fs.mkdirSync(squadDir, { recursive: true }); + + // Skill A in .copilot/skills/ + fs.mkdirSync(path.join(sourceDir, '.copilot', 'skills', 'copilot-skill'), { recursive: true }); + fs.writeFileSync(path.join(sourceDir, '.copilot', 'skills', 'copilot-skill', 'SKILL.md'), + '---\nname: copilot-skill\n---\n\nCopilot content.\n'); + + // Skill B in .squad/skills/ + fs.mkdirSync(path.join(squadDir, 'skills', 'squad-skill'), { recursive: true }); + fs.writeFileSync(path.join(squadDir, 'skills', 'squad-skill', 'SKILL.md'), + '---\nname: squad-skill\n---\n\nSquad content.\n'); + + // Set up child repo pointing to source + const childSquad = path.join(childDir, '.squad'); + fs.mkdirSync(childSquad, { recursive: true }); + fs.writeFileSync(path.join(childSquad, 'upstream.json'), JSON.stringify({ + upstreams: [{ name: 'dual', type: 'local', source: sourceDir, added_at: new Date().toISOString(), last_synced: null }], + })); + + const result = resolveUpstreams(childSquad)!; + const dual = result.upstreams.find(u => u.name === 'dual')!; + expect(dual.skills.length).toBe(2); + const names = dual.skills.map(s => s.name).sort(); + expect(names).toEqual(['copilot-skill', 'squad-skill']); + } finally { + teardown(); + } + }); + + it('deduplicates by name with .copilot/ winning', () => { + setup(); + try { + const squadDir = path.join(sourceDir, '.squad'); + fs.mkdirSync(squadDir, { recursive: true }); + + // Same skill in both dirs + fs.mkdirSync(path.join(sourceDir, '.copilot', 'skills', 'shared'), { recursive: true }); + fs.writeFileSync(path.join(sourceDir, '.copilot', 'skills', 'shared', 'SKILL.md'), + '---\nname: shared\n---\n\nCopilot version wins.\n'); + + fs.mkdirSync(path.join(squadDir, 'skills', 'shared'), { recursive: true }); + fs.writeFileSync(path.join(squadDir, 'skills', 'shared', 'SKILL.md'), + '---\nname: shared\n---\n\nSquad version loses.\n'); + + const childSquad = path.join(childDir, '.squad'); + fs.mkdirSync(childSquad, { recursive: true }); + fs.writeFileSync(path.join(childSquad, 'upstream.json'), JSON.stringify({ + upstreams: [{ name: 'dedup', type: 'local', source: sourceDir, added_at: new Date().toISOString(), last_synced: null }], + })); + + const result = resolveUpstreams(childSquad)!; + const dedup = result.upstreams.find(u => u.name === 'dedup')!; + expect(dedup.skills).toHaveLength(1); + expect(dedup.skills[0].content).toContain('Copilot version wins'); + } finally { + teardown(); + } + }); + + it('handles missing directories gracefully', () => { + setup(); + try { + const squadDir = path.join(sourceDir, '.squad'); + fs.mkdirSync(squadDir, { recursive: true }); + // No skills directories exist at all + + const childSquad = path.join(childDir, '.squad'); + fs.mkdirSync(childSquad, { recursive: true }); + fs.writeFileSync(path.join(childSquad, 'upstream.json'), JSON.stringify({ + upstreams: [{ name: 'empty', type: 'local', source: sourceDir, added_at: new Date().toISOString(), last_synced: null }], + })); + + const result = resolveUpstreams(childSquad)!; + const empty = result.upstreams.find(u => u.name === 'empty')!; + expect(empty.skills).toHaveLength(0); + } finally { + teardown(); + } + }); +});