From df3517d5aebd7dc0c5e046f2ac4c948834f27d22 Mon Sep 17 00:00:00 2001 From: Christopher Date: Wed, 28 Jan 2026 11:22:14 +1100 Subject: [PATCH 1/9] feat(utils): add hash utility for deterministic short IDs Add getShortId() function that generates a deterministic 6-character hexadecimal hash from a string input using SHA-256. This will be used for local path disambiguation when multiple plugins have skills with the same folder name and plugin name. Co-Authored-By: Claude Opus 4.5 --- src/utils/hash.ts | 15 ++++++++ tests/unit/utils/hash.test.ts | 64 +++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 src/utils/hash.ts create mode 100644 tests/unit/utils/hash.test.ts diff --git a/src/utils/hash.ts b/src/utils/hash.ts new file mode 100644 index 00000000..61f57a84 --- /dev/null +++ b/src/utils/hash.ts @@ -0,0 +1,15 @@ +import { createHash } from 'node:crypto'; + +/** + * Generate a deterministic 6-character short ID from a string + * + * This is used for local path disambiguation when multiple plugins have + * skills with the same folder name AND the same plugin name. The short ID + * provides a stable identifier derived from the full path. + * + * @param input - The string to hash (typically a file path) + * @returns A 6-character hexadecimal string + */ +export function getShortId(input: string): string { + return createHash('sha256').update(input).digest('hex').substring(0, 6); +} diff --git a/tests/unit/utils/hash.test.ts b/tests/unit/utils/hash.test.ts new file mode 100644 index 00000000..5fbf0c45 --- /dev/null +++ b/tests/unit/utils/hash.test.ts @@ -0,0 +1,64 @@ +import { describe, it, expect } from 'bun:test'; +import { getShortId } from '../../../src/utils/hash.js'; + +describe('getShortId', () => { + it('should return a 6-character string', () => { + const result = getShortId('test-input'); + expect(result.length).toBe(6); + }); + + it('should return only hexadecimal characters', () => { + const result = getShortId('some/path/to/plugin'); + expect(result).toMatch(/^[0-9a-f]{6}$/); + }); + + it('should be deterministic - same input produces same output', () => { + const input = '/home/user/projects/my-plugin'; + const result1 = getShortId(input); + const result2 = getShortId(input); + expect(result1).toBe(result2); + }); + + it('should produce different outputs for different inputs', () => { + const result1 = getShortId('/path/to/plugin-a'); + const result2 = getShortId('/path/to/plugin-b'); + expect(result1).not.toBe(result2); + }); + + it('should handle empty string', () => { + const result = getShortId(''); + expect(result.length).toBe(6); + expect(result).toMatch(/^[0-9a-f]{6}$/); + }); + + it('should handle special characters in path', () => { + const result = getShortId('/path/with spaces/and-dashes_underscores.dots'); + expect(result.length).toBe(6); + expect(result).toMatch(/^[0-9a-f]{6}$/); + }); + + it('should handle unicode characters', () => { + const result = getShortId('/path/with/Γ©mojis/πŸš€/and/ζ—₯本θͺž'); + expect(result.length).toBe(6); + expect(result).toMatch(/^[0-9a-f]{6}$/); + }); + + it('should produce known hash for predictable testing', () => { + // SHA-256 of 'test' starts with '9f86d0...' + const result = getShortId('test'); + expect(result).toBe('9f86d0'); + }); + + it('should handle very long strings', () => { + const longPath = '/very/long/path/' + 'a'.repeat(1000); + const result = getShortId(longPath); + expect(result.length).toBe(6); + expect(result).toMatch(/^[0-9a-f]{6}$/); + }); + + it('should differentiate similar paths', () => { + const path1 = '/home/user/project1/plugins/skill'; + const path2 = '/home/user/project2/plugins/skill'; + expect(getShortId(path1)).not.toBe(getShortId(path2)); + }); +}); From 1d41ac610cf66f1cb2d1a8e5a7ea889f4bf1fa38 Mon Sep 17 00:00:00 2001 From: Christopher Date: Wed, 28 Jan 2026 11:26:20 +1100 Subject: [PATCH 2/9] feat(utils): add source parser to extract org from GitHub URLs Add extractOrgFromSource() function that extracts the organization/owner from various GitHub source URL formats. This is used for skill name disambiguation when multiple plugins have skills with the same folder name and plugin name. Supports: - github:org/repo and github:org/repo#branch - gh:org/repo and gh:org/repo#branch - https://github.com/org/repo and full URLs with paths - github.com/org/repo (no protocol) - org/repo shorthand format - Returns null for local paths (caller uses hash fallback) Co-Authored-By: Claude Opus 4.5 --- src/utils/source-parser.ts | 146 ++++++++++++++++++++ tests/unit/utils/source-parser.test.ts | 180 +++++++++++++++++++++++++ 2 files changed, 326 insertions(+) create mode 100644 src/utils/source-parser.ts create mode 100644 tests/unit/utils/source-parser.test.ts diff --git a/src/utils/source-parser.ts b/src/utils/source-parser.ts new file mode 100644 index 00000000..dd200c45 --- /dev/null +++ b/src/utils/source-parser.ts @@ -0,0 +1,146 @@ +/** + * Utility for parsing plugin source strings to extract organization/owner information + * + * This is used for skill name disambiguation when multiple plugins have skills + * with the same folder name AND the same plugin name. For GitHub sources, we + * extract the org name. For local paths, the caller should fall back to using + * the hash utility. + */ + +/** + * Extract the organization/owner from a GitHub source URL + * + * Supports the following formats: + * - `github:org/repo` β†’ org + * - `github:org/repo#branch` β†’ org + * - `gh:org/repo` β†’ org + * - `gh:org/repo#branch` β†’ org + * - `https://github.com/org/repo` β†’ org + * - `https://github.com/org/repo/tree/branch/path` β†’ org + * - `github.com/org/repo` β†’ org + * - `org/repo` (shorthand) β†’ org + * - Local paths (starting with `.`, `/`, or Windows drive) β†’ null + * - Invalid or empty org β†’ null + * + * @param source - The plugin source string + * @returns The organization/owner name, or null for local paths or invalid sources + */ +export function extractOrgFromSource(source: string): string | null { + // Handle empty or whitespace-only input + if (!source || source.trim() === '') { + return null; + } + + const trimmedSource = source.trim(); + + // Check for local paths first + if (isLocalPath(trimmedSource)) { + return null; + } + + // Handle github: prefix (e.g., github:org/repo or github:org/repo#branch) + if (trimmedSource.startsWith('github:')) { + return parseOrgFromPrefix(trimmedSource.slice('github:'.length)); + } + + // Handle gh: prefix (e.g., gh:org/repo or gh:org/repo#branch) + if (trimmedSource.startsWith('gh:')) { + return parseOrgFromPrefix(trimmedSource.slice('gh:'.length)); + } + + // Handle full GitHub URLs (https://github.com/org/repo or github.com/org/repo) + const urlMatch = trimmedSource.match( + /^(?:https?:\/\/)?(?:www\.)?github\.com\/([^/]+)(?:\/|$)/ + ); + if (urlMatch && urlMatch[1]) { + return validateOrg(urlMatch[1]); + } + + // Handle shorthand format (org/repo) - must not look like a local path + if (trimmedSource.includes('/') && !trimmedSource.includes('://')) { + const firstSlashIndex = trimmedSource.indexOf('/'); + const potentialOrg = trimmedSource.slice(0, firstSlashIndex); + return validateOrg(potentialOrg); + } + + return null; +} + +/** + * Check if a source string represents a local path + */ +function isLocalPath(source: string): boolean { + // Starts with . (relative path) + if (source.startsWith('.')) { + return true; + } + + // Starts with / (Unix absolute path) + if (source.startsWith('/')) { + return true; + } + + // Windows absolute path (e.g., C:\, D:\) + if (/^[a-zA-Z]:[\\/]/.test(source)) { + return true; + } + + // Contains backslash (Windows path separator) + if (source.includes('\\')) { + return true; + } + + return false; +} + +/** + * Parse org from a prefix format (org/repo or org/repo#branch) + */ +function parseOrgFromPrefix(prefixContent: string): string | null { + // Remove branch suffix if present (e.g., org/repo#branch β†’ org/repo) + const withoutBranch = prefixContent.split('#')[0] || ''; + + // Extract org (everything before the first /) + const slashIndex = withoutBranch.indexOf('/'); + if (slashIndex === -1) { + // No slash means no valid org/repo format + return null; + } + + const org = withoutBranch.slice(0, slashIndex); + return validateOrg(org); +} + +/** + * Validate that an org string is valid (non-empty, valid characters) + * GitHub usernames/orgs: alphanumeric, hyphens, can't start/end with hyphen + */ +function validateOrg(org: string): string | null { + if (!org || org.trim() === '') { + return null; + } + + const trimmedOrg = org.trim(); + + // GitHub org/username rules: + // - Can contain alphanumeric characters and hyphens + // - Cannot start or end with a hyphen + // - Cannot have consecutive hyphens + // - Must be 1-39 characters + const validOrgPattern = /^[a-zA-Z0-9](?:[a-zA-Z0-9-]*[a-zA-Z0-9])?$/; + + if (!validOrgPattern.test(trimmedOrg)) { + // Check if it's a single character (which is valid) + if (trimmedOrg.length === 1 && /^[a-zA-Z0-9]$/.test(trimmedOrg)) { + return trimmedOrg; + } + return null; + } + + // Check for consecutive hyphens + if (trimmedOrg.includes('--')) { + return null; + } + + return trimmedOrg; +} diff --git a/tests/unit/utils/source-parser.test.ts b/tests/unit/utils/source-parser.test.ts new file mode 100644 index 00000000..698dcf14 --- /dev/null +++ b/tests/unit/utils/source-parser.test.ts @@ -0,0 +1,180 @@ +import { describe, it, expect } from 'bun:test'; +import { extractOrgFromSource } from '../../../src/utils/source-parser.js'; + +describe('extractOrgFromSource', () => { + describe('github: prefix format', () => { + it('should extract org from github:org/repo', () => { + expect(extractOrgFromSource('github:anthropic/repo')).toBe('anthropic'); + }); + + it('should extract org from github:org/repo#branch', () => { + expect(extractOrgFromSource('github:anthropic/repo#main')).toBe('anthropic'); + expect(extractOrgFromSource('github:anthropic/repo#feature/my-branch')).toBe('anthropic'); + }); + + it('should handle hyphenated org names', () => { + expect(extractOrgFromSource('github:my-org/repo')).toBe('my-org'); + expect(extractOrgFromSource('github:my-org/repo#branch')).toBe('my-org'); + }); + + it('should handle numeric org names', () => { + expect(extractOrgFromSource('github:org123/repo')).toBe('org123'); + }); + + it('should handle single character org names', () => { + expect(extractOrgFromSource('github:a/repo')).toBe('a'); + }); + }); + + describe('gh: prefix format', () => { + it('should extract org from gh:org/repo', () => { + expect(extractOrgFromSource('gh:anthropic/repo')).toBe('anthropic'); + }); + + it('should extract org from gh:org/repo#branch', () => { + expect(extractOrgFromSource('gh:anthropic/repo#develop')).toBe('anthropic'); + }); + }); + + describe('full GitHub URL format', () => { + it('should extract org from https://github.com/org/repo', () => { + expect(extractOrgFromSource('https://github.com/anthropic/claude-code')).toBe('anthropic'); + }); + + it('should extract org from http://github.com/org/repo', () => { + expect(extractOrgFromSource('http://github.com/anthropic/repo')).toBe('anthropic'); + }); + + it('should extract org from https://www.github.com/org/repo', () => { + expect(extractOrgFromSource('https://www.github.com/EntityProcess/allagents')).toBe('EntityProcess'); + }); + + it('should extract org from github.com/org/repo (no protocol)', () => { + expect(extractOrgFromSource('github.com/anthropic/repo')).toBe('anthropic'); + }); + + it('should extract org from URLs with tree/branch/path', () => { + expect(extractOrgFromSource('https://github.com/anthropic/repo/tree/main/plugins')).toBe('anthropic'); + }); + }); + + describe('shorthand format (org/repo)', () => { + it('should extract org from owner/repo', () => { + expect(extractOrgFromSource('anthropic/claude-plugins-official')).toBe('anthropic'); + }); + + it('should extract org from owner/repo/subpath', () => { + expect(extractOrgFromSource('anthropic/repo/plugins/code-review')).toBe('anthropic'); + }); + + it('should handle dots in repo names', () => { + expect(extractOrgFromSource('WiseTechGlobal/WTG.AI.Prompts')).toBe('WiseTechGlobal'); + }); + }); + + describe('local paths - should return null', () => { + it('should return null for relative paths starting with ./', () => { + expect(extractOrgFromSource('./local/path')).toBeNull(); + }); + + it('should return null for relative paths starting with ../', () => { + expect(extractOrgFromSource('../parent/path')).toBeNull(); + }); + + it('should return null for absolute Unix paths', () => { + expect(extractOrgFromSource('/home/user/plugins/skill')).toBeNull(); + }); + + it('should return null for Windows absolute paths', () => { + expect(extractOrgFromSource('C:\\Users\\test\\plugins')).toBeNull(); + expect(extractOrgFromSource('D:/Projects/plugins')).toBeNull(); + }); + + it('should return null for paths with backslashes', () => { + expect(extractOrgFromSource('local\\path\\to\\plugin')).toBeNull(); + }); + + it('should return null for current directory', () => { + expect(extractOrgFromSource('.')).toBeNull(); + }); + + it('should return null for parent directory', () => { + expect(extractOrgFromSource('..')).toBeNull(); + }); + }); + + describe('invalid or empty input - should return null', () => { + it('should return null for empty string', () => { + expect(extractOrgFromSource('')).toBeNull(); + }); + + it('should return null for whitespace-only string', () => { + expect(extractOrgFromSource(' ')).toBeNull(); + expect(extractOrgFromSource('\t')).toBeNull(); + expect(extractOrgFromSource('\n')).toBeNull(); + }); + + it('should return null for github: with no repo', () => { + expect(extractOrgFromSource('github:')).toBeNull(); + }); + + it('should return null for github: with empty org', () => { + expect(extractOrgFromSource('github:/repo')).toBeNull(); + }); + + it('should return null for github: with only org (no slash)', () => { + expect(extractOrgFromSource('github:anthropic')).toBeNull(); + }); + + it('should return null for invalid org starting with hyphen', () => { + expect(extractOrgFromSource('github:-invalid/repo')).toBeNull(); + }); + + it('should return null for invalid org ending with hyphen', () => { + expect(extractOrgFromSource('github:invalid-/repo')).toBeNull(); + }); + + it('should return null for org with consecutive hyphens', () => { + expect(extractOrgFromSource('github:in--valid/repo')).toBeNull(); + }); + + it('should return null for org with special characters', () => { + expect(extractOrgFromSource('github:org@name/repo')).toBeNull(); + expect(extractOrgFromSource('github:org.name/repo')).toBeNull(); + expect(extractOrgFromSource('github:org_name/repo')).toBeNull(); + }); + + it('should return null for string with no slash', () => { + expect(extractOrgFromSource('anthropic')).toBeNull(); + }); + }); + + describe('edge cases', () => { + it('should handle whitespace around input', () => { + expect(extractOrgFromSource(' github:anthropic/repo ')).toBe('anthropic'); + }); + + it('should handle multiple # in branch name', () => { + expect(extractOrgFromSource('github:anthropic/repo#branch#with#hashes')).toBe('anthropic'); + }); + + it('should handle empty branch after #', () => { + expect(extractOrgFromSource('github:anthropic/repo#')).toBe('anthropic'); + }); + + it('should be case-sensitive for org names', () => { + expect(extractOrgFromSource('github:Anthropic/repo')).toBe('Anthropic'); + expect(extractOrgFromSource('github:ANTHROPIC/repo')).toBe('ANTHROPIC'); + }); + + it('should handle long org names', () => { + const longOrg = 'a'.repeat(39); + expect(extractOrgFromSource(`github:${longOrg}/repo`)).toBe(longOrg); + }); + + it('should handle org names with numbers', () => { + expect(extractOrgFromSource('github:123org/repo')).toBe('123org'); + expect(extractOrgFromSource('github:org456/repo')).toBe('org456'); + }); + }); +}); From 1a2c6aa64696a2a36679779ce4ade2cf47b704b7 Mon Sep 17 00:00:00 2001 From: Christopher Date: Wed, 28 Jan 2026 11:31:50 +1100 Subject: [PATCH 3/9] feat(utils): add skill name resolver for duplicate handling Implements three-tier naming resolution for skills with duplicate names: 1. No conflict - use skill folder name as-is 2. Folder name conflicts - qualify with plugin name: {plugin}_{skill} 3. Both conflict - add org/hash prefix: {orgOrUuid}_{plugin}_{skill} Uses getShortId() for local paths and extractOrgFromSource() for GitHub. Co-Authored-By: Claude Opus 4.5 --- src/utils/skill-name-resolver.ts | 161 ++++++ tests/unit/utils/skill-name-resolver.test.ts | 514 +++++++++++++++++++ 2 files changed, 675 insertions(+) create mode 100644 src/utils/skill-name-resolver.ts create mode 100644 tests/unit/utils/skill-name-resolver.test.ts diff --git a/src/utils/skill-name-resolver.ts b/src/utils/skill-name-resolver.ts new file mode 100644 index 00000000..1198c984 --- /dev/null +++ b/src/utils/skill-name-resolver.ts @@ -0,0 +1,161 @@ +/** + * Skill name resolver for handling duplicate skill names across plugins + * + * Resolution rules: + * 1. No conflict - Use skill folder name as-is + * 2. Skill folder name conflicts across plugins - Qualify with plugin name: {plugin}_{skill} + * 3. Both plugin name AND skill folder name conflict - Add org/UUID prefix: {orgOrUuid}_{plugin}_{skill} + */ + +import { getShortId } from './hash.js'; +import { extractOrgFromSource } from './source-parser.js'; + +/** + * Input skill entry for name resolution + */ +export interface SkillEntry { + /** Skill folder name (e.g., "my-skill") */ + folderName: string; + /** Plugin name from plugin.json (e.g., "my-plugin") */ + pluginName: string; + /** Plugin source - GitHub URL/shorthand or local path */ + pluginSource: string; +} + +/** + * Resolved skill name entry + */ +export interface ResolvedSkillName { + /** Original skill entry */ + original: SkillEntry; + /** Resolved unique name for the skill */ + resolvedName: string; + /** Indicates if the name was modified from the original folder name */ + wasRenamed: boolean; +} + +/** + * Result of skill name resolution + */ +export interface SkillNameResolutionResult { + /** Array of resolved skill names */ + resolved: ResolvedSkillName[]; + /** Map from original key to resolved name for quick lookup */ + nameMap: Map; +} + +/** + * Generate a unique key for a skill entry + * Used for internal mapping and deduplication + */ +export function getSkillKey(entry: SkillEntry): string { + return `${entry.pluginSource}::${entry.pluginName}::${entry.folderName}`; +} + +/** + * Get the disambiguator prefix for a plugin source + * Returns the GitHub org name for GitHub sources, or a 6-char hash for local paths + */ +export function getDisambiguatorPrefix(pluginSource: string): string { + const org = extractOrgFromSource(pluginSource); + if (org) { + return org; + } + // For local paths, use a 6-char hash of the source + return getShortId(pluginSource); +} + +/** + * Resolve skill names to ensure uniqueness across all plugins + * + * @param skills - Array of skill entries to resolve + * @returns Resolution result with resolved names and lookup map + */ +export function resolveSkillNames( + skills: SkillEntry[], +): SkillNameResolutionResult { + const resolved: ResolvedSkillName[] = []; + const nameMap = new Map(); + + // Step 1: Group skills by folder name + const byFolderName = new Map(); + for (const skill of skills) { + const existing = byFolderName.get(skill.folderName) || []; + existing.push(skill); + byFolderName.set(skill.folderName, existing); + } + + // Step 2: Process each group + for (const [folderName, group] of byFolderName) { + if (group.length === 1) { + // No conflict - use folder name as-is + const skill = group[0]; + if (skill) { + const resolvedEntry: ResolvedSkillName = { + original: skill, + resolvedName: folderName, + wasRenamed: false, + }; + resolved.push(resolvedEntry); + nameMap.set(getSkillKey(skill), folderName); + } + } else { + // Conflict detected - need to qualify names + // Check if plugin names are unique within this group + const byPluginName = new Map(); + for (const skill of group) { + const existing = byPluginName.get(skill.pluginName) || []; + existing.push(skill); + byPluginName.set(skill.pluginName, existing); + } + + // Determine if plugin names are unique + const hasPluginNameConflict = Array.from(byPluginName.values()).some( + (pluginGroup) => pluginGroup.length > 1, + ); + + if (!hasPluginNameConflict) { + // Plugin names are unique - use {plugin}_{skill} + for (const skill of group) { + const resolvedName = `${skill.pluginName}_${folderName}`; + const resolvedEntry: ResolvedSkillName = { + original: skill, + resolvedName, + wasRenamed: true, + }; + resolved.push(resolvedEntry); + nameMap.set(getSkillKey(skill), resolvedName); + } + } else { + // Plugin names also conflict - need org/hash prefix + for (const skill of group) { + const prefix = getDisambiguatorPrefix(skill.pluginSource); + const resolvedName = `${prefix}_${skill.pluginName}_${folderName}`; + const resolvedEntry: ResolvedSkillName = { + original: skill, + resolvedName, + wasRenamed: true, + }; + resolved.push(resolvedEntry); + nameMap.set(getSkillKey(skill), resolvedName); + } + } + } + } + + return { resolved, nameMap }; +} + +/** + * Get the resolved name for a specific skill + * + * @param result - The resolution result from resolveSkillNames + * @param skill - The skill entry to look up + * @returns The resolved name, or undefined if not found + */ +export function getResolvedName( + result: SkillNameResolutionResult, + skill: SkillEntry, +): string | undefined { + return result.nameMap.get(getSkillKey(skill)); +} diff --git a/tests/unit/utils/skill-name-resolver.test.ts b/tests/unit/utils/skill-name-resolver.test.ts new file mode 100644 index 00000000..0723edfc --- /dev/null +++ b/tests/unit/utils/skill-name-resolver.test.ts @@ -0,0 +1,514 @@ +import { describe, it, expect } from 'bun:test'; +import { + resolveSkillNames, + getResolvedName, + getSkillKey, + getDisambiguatorPrefix, + type SkillEntry, +} from '../../../src/utils/skill-name-resolver.js'; + +describe('getSkillKey', () => { + it('should generate a unique key from skill entry', () => { + const entry: SkillEntry = { + folderName: 'my-skill', + pluginName: 'my-plugin', + pluginSource: 'github:org/repo', + }; + const key = getSkillKey(entry); + expect(key).toBe('github:org/repo::my-plugin::my-skill'); + }); + + it('should generate different keys for different entries', () => { + const entry1: SkillEntry = { + folderName: 'skill', + pluginName: 'plugin-a', + pluginSource: 'github:org/repo-a', + }; + const entry2: SkillEntry = { + folderName: 'skill', + pluginName: 'plugin-b', + pluginSource: 'github:org/repo-b', + }; + expect(getSkillKey(entry1)).not.toBe(getSkillKey(entry2)); + }); +}); + +describe('getDisambiguatorPrefix', () => { + it('should extract org from GitHub URL', () => { + expect(getDisambiguatorPrefix('github:anthropic/plugins')).toBe( + 'anthropic', + ); + }); + + it('should extract org from GitHub shorthand', () => { + expect(getDisambiguatorPrefix('my-org/my-repo')).toBe('my-org'); + }); + + it('should extract org from full GitHub URL', () => { + expect( + getDisambiguatorPrefix('https://github.com/acme-corp/tools'), + ).toBe('acme-corp'); + }); + + it('should return hash for local path starting with /', () => { + const prefix = getDisambiguatorPrefix('/home/user/plugins/my-plugin'); + expect(prefix).toMatch(/^[0-9a-f]{6}$/); + }); + + it('should return hash for local path starting with .', () => { + const prefix = getDisambiguatorPrefix('./local/plugin'); + expect(prefix).toMatch(/^[0-9a-f]{6}$/); + }); + + it('should return hash for Windows path', () => { + const prefix = getDisambiguatorPrefix('C:\\Users\\dev\\plugins'); + expect(prefix).toMatch(/^[0-9a-f]{6}$/); + }); + + it('should return deterministic hash for same local path', () => { + const path = '/home/user/my-plugin'; + const prefix1 = getDisambiguatorPrefix(path); + const prefix2 = getDisambiguatorPrefix(path); + expect(prefix1).toBe(prefix2); + }); + + it('should return different hashes for different local paths', () => { + const prefix1 = getDisambiguatorPrefix('/path/to/plugin-a'); + const prefix2 = getDisambiguatorPrefix('/path/to/plugin-b'); + expect(prefix1).not.toBe(prefix2); + }); +}); + +describe('resolveSkillNames', () => { + describe('no conflicts', () => { + it('should use folder names as-is when all unique', () => { + const skills: SkillEntry[] = [ + { + folderName: 'skill-a', + pluginName: 'plugin-1', + pluginSource: 'github:org/repo-1', + }, + { + folderName: 'skill-b', + pluginName: 'plugin-2', + pluginSource: 'github:org/repo-2', + }, + { + folderName: 'skill-c', + pluginName: 'plugin-3', + pluginSource: 'github:org/repo-3', + }, + ]; + + const result = resolveSkillNames(skills); + + expect(result.resolved).toHaveLength(3); + expect(result.resolved[0]?.resolvedName).toBe('skill-a'); + expect(result.resolved[0]?.wasRenamed).toBe(false); + expect(result.resolved[1]?.resolvedName).toBe('skill-b'); + expect(result.resolved[1]?.wasRenamed).toBe(false); + expect(result.resolved[2]?.resolvedName).toBe('skill-c'); + expect(result.resolved[2]?.wasRenamed).toBe(false); + }); + + it('should handle single skill', () => { + const skills: SkillEntry[] = [ + { + folderName: 'only-skill', + pluginName: 'single-plugin', + pluginSource: 'github:solo/repo', + }, + ]; + + const result = resolveSkillNames(skills); + + expect(result.resolved).toHaveLength(1); + expect(result.resolved[0]?.resolvedName).toBe('only-skill'); + expect(result.resolved[0]?.wasRenamed).toBe(false); + }); + + it('should handle empty array', () => { + const skills: SkillEntry[] = []; + const result = resolveSkillNames(skills); + + expect(result.resolved).toHaveLength(0); + expect(result.nameMap.size).toBe(0); + }); + }); + + describe('folder name conflicts with unique plugin names', () => { + it('should qualify with plugin name when folder names conflict', () => { + const skills: SkillEntry[] = [ + { + folderName: 'commit', + pluginName: 'git-tools', + pluginSource: 'github:team-a/plugins', + }, + { + folderName: 'commit', + pluginName: 'dev-utils', + pluginSource: 'github:team-b/plugins', + }, + ]; + + const result = resolveSkillNames(skills); + + expect(result.resolved).toHaveLength(2); + expect(result.resolved[0]?.resolvedName).toBe('git-tools_commit'); + expect(result.resolved[0]?.wasRenamed).toBe(true); + expect(result.resolved[1]?.resolvedName).toBe('dev-utils_commit'); + expect(result.resolved[1]?.wasRenamed).toBe(true); + }); + + it('should handle multiple skills with same folder name', () => { + const skills: SkillEntry[] = [ + { + folderName: 'format', + pluginName: 'prettier-plugin', + pluginSource: 'github:formatter/prettier', + }, + { + folderName: 'format', + pluginName: 'eslint-plugin', + pluginSource: 'github:linter/eslint', + }, + { + folderName: 'format', + pluginName: 'stylelint-plugin', + pluginSource: 'github:linter/stylelint', + }, + ]; + + const result = resolveSkillNames(skills); + + expect(result.resolved).toHaveLength(3); + expect(result.resolved[0]?.resolvedName).toBe('prettier-plugin_format'); + expect(result.resolved[1]?.resolvedName).toBe('eslint-plugin_format'); + expect(result.resolved[2]?.resolvedName).toBe('stylelint-plugin_format'); + }); + + it('should mix renamed and non-renamed skills', () => { + const skills: SkillEntry[] = [ + { + folderName: 'unique-skill', + pluginName: 'plugin-a', + pluginSource: 'github:org/repo-a', + }, + { + folderName: 'duplicate', + pluginName: 'plugin-b', + pluginSource: 'github:org/repo-b', + }, + { + folderName: 'duplicate', + pluginName: 'plugin-c', + pluginSource: 'github:org/repo-c', + }, + ]; + + const result = resolveSkillNames(skills); + + expect(result.resolved).toHaveLength(3); + + const unique = result.resolved.find( + (r) => r.original.folderName === 'unique-skill', + ); + expect(unique?.resolvedName).toBe('unique-skill'); + expect(unique?.wasRenamed).toBe(false); + + const dup1 = result.resolved.find( + (r) => r.original.pluginName === 'plugin-b', + ); + expect(dup1?.resolvedName).toBe('plugin-b_duplicate'); + expect(dup1?.wasRenamed).toBe(true); + + const dup2 = result.resolved.find( + (r) => r.original.pluginName === 'plugin-c', + ); + expect(dup2?.resolvedName).toBe('plugin-c_duplicate'); + expect(dup2?.wasRenamed).toBe(true); + }); + }); + + describe('folder name AND plugin name conflicts', () => { + it('should use org prefix for GitHub sources', () => { + const skills: SkillEntry[] = [ + { + folderName: 'test', + pluginName: 'testing', + pluginSource: 'github:company-a/plugins', + }, + { + folderName: 'test', + pluginName: 'testing', + pluginSource: 'github:company-b/plugins', + }, + ]; + + const result = resolveSkillNames(skills); + + expect(result.resolved).toHaveLength(2); + expect(result.resolved[0]?.resolvedName).toBe('company-a_testing_test'); + expect(result.resolved[0]?.wasRenamed).toBe(true); + expect(result.resolved[1]?.resolvedName).toBe('company-b_testing_test'); + expect(result.resolved[1]?.wasRenamed).toBe(true); + }); + + it('should use hash prefix for local paths', () => { + const skills: SkillEntry[] = [ + { + folderName: 'build', + pluginName: 'builder', + pluginSource: '/home/user/project-a/plugins', + }, + { + folderName: 'build', + pluginName: 'builder', + pluginSource: '/home/user/project-b/plugins', + }, + ]; + + const result = resolveSkillNames(skills); + + expect(result.resolved).toHaveLength(2); + // Both should have 6-char hash prefix + expect(result.resolved[0]?.resolvedName).toMatch( + /^[0-9a-f]{6}_builder_build$/, + ); + expect(result.resolved[1]?.resolvedName).toMatch( + /^[0-9a-f]{6}_builder_build$/, + ); + // And they should be different + expect(result.resolved[0]?.resolvedName).not.toBe( + result.resolved[1]?.resolvedName, + ); + }); + + it('should handle mixed GitHub and local sources with same names', () => { + const skills: SkillEntry[] = [ + { + folderName: 'deploy', + pluginName: 'infra', + pluginSource: 'github:acme-corp/tools', + }, + { + folderName: 'deploy', + pluginName: 'infra', + pluginSource: '/local/dev/plugins', + }, + ]; + + const result = resolveSkillNames(skills); + + expect(result.resolved).toHaveLength(2); + expect(result.resolved[0]?.resolvedName).toBe('acme-corp_infra_deploy'); + expect(result.resolved[1]?.resolvedName).toMatch( + /^[0-9a-f]{6}_infra_deploy$/, + ); + }); + + it('should handle three-way plugin name conflict', () => { + const skills: SkillEntry[] = [ + { + folderName: 'lint', + pluginName: 'quality', + pluginSource: 'github:org-1/repo', + }, + { + folderName: 'lint', + pluginName: 'quality', + pluginSource: 'github:org-2/repo', + }, + { + folderName: 'lint', + pluginName: 'quality', + pluginSource: 'github:org-3/repo', + }, + ]; + + const result = resolveSkillNames(skills); + + expect(result.resolved).toHaveLength(3); + expect(result.resolved[0]?.resolvedName).toBe('org-1_quality_lint'); + expect(result.resolved[1]?.resolvedName).toBe('org-2_quality_lint'); + expect(result.resolved[2]?.resolvedName).toBe('org-3_quality_lint'); + }); + + it('should handle partial plugin name conflicts', () => { + // Two skills with same folder name, where two have same plugin name + const skills: SkillEntry[] = [ + { + folderName: 'check', + pluginName: 'validator', + pluginSource: 'github:alpha/tools', + }, + { + folderName: 'check', + pluginName: 'validator', + pluginSource: 'github:beta/tools', + }, + { + folderName: 'check', + pluginName: 'verifier', + pluginSource: 'github:gamma/tools', + }, + ]; + + const result = resolveSkillNames(skills); + + expect(result.resolved).toHaveLength(3); + // All should have org prefix because there's at least one plugin name conflict + expect(result.resolved[0]?.resolvedName).toBe('alpha_validator_check'); + expect(result.resolved[1]?.resolvedName).toBe('beta_validator_check'); + expect(result.resolved[2]?.resolvedName).toBe('gamma_verifier_check'); + }); + }); + + describe('nameMap', () => { + it('should provide correct lookup via nameMap', () => { + const skills: SkillEntry[] = [ + { + folderName: 'skill-a', + pluginName: 'plugin-1', + pluginSource: 'github:org/repo-1', + }, + { + folderName: 'skill-b', + pluginName: 'plugin-2', + pluginSource: 'github:org/repo-2', + }, + ]; + + const result = resolveSkillNames(skills); + + expect(result.nameMap.get(getSkillKey(skills[0]!))).toBe('skill-a'); + expect(result.nameMap.get(getSkillKey(skills[1]!))).toBe('skill-b'); + }); + }); +}); + +describe('getResolvedName', () => { + it('should return resolved name for existing skill', () => { + const skill: SkillEntry = { + folderName: 'my-skill', + pluginName: 'my-plugin', + pluginSource: 'github:my-org/my-repo', + }; + + const result = resolveSkillNames([skill]); + const name = getResolvedName(result, skill); + + expect(name).toBe('my-skill'); + }); + + it('should return undefined for non-existing skill', () => { + const skill1: SkillEntry = { + folderName: 'skill-1', + pluginName: 'plugin-1', + pluginSource: 'github:org/repo', + }; + const skill2: SkillEntry = { + folderName: 'skill-2', + pluginName: 'plugin-2', + pluginSource: 'github:other/repo', + }; + + const result = resolveSkillNames([skill1]); + const name = getResolvedName(result, skill2); + + expect(name).toBeUndefined(); + }); + + it('should work with renamed skills', () => { + const skills: SkillEntry[] = [ + { + folderName: 'common', + pluginName: 'alpha', + pluginSource: 'github:org/alpha', + }, + { + folderName: 'common', + pluginName: 'beta', + pluginSource: 'github:org/beta', + }, + ]; + + const result = resolveSkillNames(skills); + + expect(getResolvedName(result, skills[0]!)).toBe('alpha_common'); + expect(getResolvedName(result, skills[1]!)).toBe('beta_common'); + }); +}); + +describe('edge cases', () => { + it('should handle skills with special characters in folder names', () => { + const skills: SkillEntry[] = [ + { + folderName: 'my-skill-v2', + pluginName: 'plugin', + pluginSource: 'github:org/repo', + }, + ]; + + const result = resolveSkillNames(skills); + expect(result.resolved[0]?.resolvedName).toBe('my-skill-v2'); + }); + + it('should handle GitHub URLs with branches', () => { + const skills: SkillEntry[] = [ + { + folderName: 'skill', + pluginName: 'plugin', + pluginSource: 'github:org/repo#feature-branch', + }, + { + folderName: 'skill', + pluginName: 'plugin', + pluginSource: 'github:other-org/repo#main', + }, + ]; + + const result = resolveSkillNames(skills); + + expect(result.resolved).toHaveLength(2); + expect(result.resolved[0]?.resolvedName).toBe('org_plugin_skill'); + expect(result.resolved[1]?.resolvedName).toBe('other-org_plugin_skill'); + }); + + it('should handle full GitHub URLs', () => { + const skills: SkillEntry[] = [ + { + folderName: 'utils', + pluginName: 'helpers', + pluginSource: 'https://github.com/enterprise/tools', + }, + { + folderName: 'utils', + pluginName: 'helpers', + pluginSource: 'https://github.com/community/tools', + }, + ]; + + const result = resolveSkillNames(skills); + + expect(result.resolved[0]?.resolvedName).toBe('enterprise_helpers_utils'); + expect(result.resolved[1]?.resolvedName).toBe('community_helpers_utils'); + }); + + it('should preserve original entry in resolved result', () => { + const skill: SkillEntry = { + folderName: 'test-skill', + pluginName: 'test-plugin', + pluginSource: 'github:test-org/test-repo', + }; + + const result = resolveSkillNames([skill]); + + expect(result.resolved[0]?.original).toBe(skill); + expect(result.resolved[0]?.original.folderName).toBe('test-skill'); + expect(result.resolved[0]?.original.pluginName).toBe('test-plugin'); + expect(result.resolved[0]?.original.pluginSource).toBe( + 'github:test-org/test-repo', + ); + }); +}); From 770db2c8dfa9e3aaab6212d35b2e6c49b8975d57 Mon Sep 17 00:00:00 2001 From: Christopher Date: Wed, 28 Jan 2026 11:42:12 +1100 Subject: [PATCH 4/9] feat(sync): implement two-pass skill name resolution Refactors the sync process to collect all skills from all plugins before copying, enabling automatic duplicate skill name handling: - Add SkillCopyOptions and PluginCopyOptions interfaces to transform.ts - Add collectPluginSkills() to gather skill entries from plugins - Add getPluginName() to read plugin name from plugin.json or fallback to directory name - Modify copySkills() to accept optional skillNameMap for resolved names - Add collectAllSkills() and buildPluginSkillNameMaps() to sync.ts - Update syncWorkspace() to use two-pass resolution: Pass 1: Collect all skills from validated plugins Pass 2: Copy skills using resolved names from the skill name resolver When skill folder names conflict: - Unique plugins qualify with plugin name: {plugin}_{skill} - Same plugin names add org/hash prefix: {org}_{plugin}_{skill} Backward compatible: when no conflicts, skills use original folder names. Co-Authored-By: Claude Opus 4.5 --- src/core/plugin.ts | 31 +- src/core/sync.ts | 120 +++++++- src/core/transform.ts | 90 +++++- tests/unit/core/skill-resolution.test.ts | 372 +++++++++++++++++++++++ 4 files changed, 595 insertions(+), 18 deletions(-) create mode 100644 tests/unit/core/skill-resolution.test.ts diff --git a/src/core/plugin.ts b/src/core/plugin.ts index e460ddcb..5138741b 100644 --- a/src/core/plugin.ts +++ b/src/core/plugin.ts @@ -1,12 +1,13 @@ -import { mkdir, readdir, stat } from 'node:fs/promises'; +import { mkdir, readdir, stat, readFile } from 'node:fs/promises'; import { existsSync } from 'node:fs'; -import { dirname, join, resolve } from 'node:path'; +import { basename, dirname, join, resolve } from 'node:path'; import { execa } from 'execa'; import { parseGitHubUrl, getPluginCachePath, validatePluginSource, } from '../utils/plugin-path.js'; +import { PluginManifestSchema } from '../models/plugin-config.js'; /** * Information about a cached plugin @@ -291,3 +292,29 @@ export async function updateCachedPlugins( return results; } + +/** + * Get the plugin name from plugin.json or fallback to directory name + * @param pluginPath - Resolved path to the plugin directory + * @returns The plugin name + */ +export async function getPluginName(pluginPath: string): Promise { + const manifestPath = join(pluginPath, 'plugin.json'); + + if (existsSync(manifestPath)) { + try { + const content = await readFile(manifestPath, 'utf-8'); + const manifest = JSON.parse(content); + const result = PluginManifestSchema.safeParse(manifest); + + if (result.success && result.data.name) { + return result.data.name; + } + } catch { + // Fall through to directory name fallback + } + } + + // Fallback to directory name + return basename(pluginPath); +} diff --git a/src/core/sync.ts b/src/core/sync.ts index 49c2fb17..3e43095c 100644 --- a/src/core/sync.ts +++ b/src/core/sync.ts @@ -13,9 +13,19 @@ import { parseGitHubUrl, parseFileSource, } from '../utils/plugin-path.js'; -import { fetchPlugin } from './plugin.js'; -import { copyPluginToWorkspace, copyWorkspaceFiles, type CopyResult } from './transform.js'; +import { fetchPlugin, getPluginName } from './plugin.js'; +import { + copyPluginToWorkspace, + copyWorkspaceFiles, + collectPluginSkills, + type CopyResult, +} from './transform.js'; import { CLIENT_MAPPINGS } from '../models/client-mapping.js'; +import { + resolveSkillNames, + getSkillKey, + type SkillEntry, +} from '../utils/skill-name-resolver.js'; import { isPluginSpec, resolvePluginSpec, @@ -658,6 +668,7 @@ async function validateAllPlugins( * @param workspacePath - Path to workspace directory * @param clients - List of clients to sync for * @param dryRun - Simulate without making changes + * @param skillNameMap - Optional map of skill folder names to resolved names * @returns Plugin sync result */ async function copyValidatedPlugin( @@ -665,6 +676,7 @@ async function copyValidatedPlugin( workspacePath: string, clients: string[], dryRun: boolean, + skillNameMap?: Map, ): Promise { const copyResults: CopyResult[] = []; @@ -674,7 +686,7 @@ async function copyValidatedPlugin( validatedPlugin.resolved, workspacePath, client as ClientType, - { dryRun }, + { dryRun, skillNameMap }, ); copyResults.push(...results); } @@ -689,6 +701,87 @@ async function copyValidatedPlugin( }; } +/** + * Collected skill information with plugin context for name resolution + */ +interface CollectedSkillEntry { + /** Skill folder name */ + folderName: string; + /** Plugin name (from plugin.json or directory name) */ + pluginName: string; + /** Plugin source reference */ + pluginSource: string; + /** Resolved plugin path */ + pluginPath: string; +} + +/** + * Collect all skills from all validated plugins + * This is the first pass of two-pass name resolution + * @param validatedPlugins - Array of validated plugins with resolved paths + * @returns Array of collected skill entries + */ +async function collectAllSkills( + validatedPlugins: ValidatedPlugin[], +): Promise { + const allSkills: CollectedSkillEntry[] = []; + + for (const plugin of validatedPlugins) { + const pluginName = await getPluginName(plugin.resolved); + const skills = await collectPluginSkills(plugin.resolved, plugin.plugin); + + for (const skill of skills) { + allSkills.push({ + folderName: skill.folderName, + pluginName, + pluginSource: plugin.plugin, + pluginPath: plugin.resolved, + }); + } + } + + return allSkills; +} + +/** + * Build skill name maps for each plugin based on resolved names + * @param allSkills - Collected skills from all plugins + * @returns Map from plugin path to skill name map (folder name -> resolved name) + */ +function buildPluginSkillNameMaps( + allSkills: CollectedSkillEntry[], +): Map> { + // Convert to SkillEntry format for resolver + const skillEntries: SkillEntry[] = allSkills.map((skill) => ({ + folderName: skill.folderName, + pluginName: skill.pluginName, + pluginSource: skill.pluginSource, + })); + + // Resolve names using the skill name resolver + const resolution = resolveSkillNames(skillEntries); + + // Build per-plugin maps + const pluginMaps = new Map>(); + + for (let i = 0; i < allSkills.length; i++) { + const skill = allSkills[i]!; + const entry = skillEntries[i]!; + const resolvedName = resolution.nameMap.get(getSkillKey(entry)); + + if (resolvedName) { + let pluginMap = pluginMaps.get(skill.pluginPath); + if (!pluginMap) { + pluginMap = new Map(); + pluginMaps.set(skill.pluginPath, pluginMap); + } + pluginMap.set(skill.folderName, resolvedName); + } + } + + return pluginMaps; +} + /** * Sync all plugins to workspace for all configured clients * @@ -816,11 +909,26 @@ export async function syncWorkspace( await selectivePurgeWorkspace(workspacePath, previousState, config.clients); } + // Step 3b: Two-pass skill name resolution + // Pass 1: Collect all skills from all plugins + const allSkills = await collectAllSkills(validatedPlugins); + + // Build per-plugin skill name maps (handles conflicts automatically) + const pluginSkillMaps = buildPluginSkillNameMaps(allSkills); + // Step 4: Copy fresh from all validated plugins + // Pass 2: Copy skills using resolved names const pluginResults = await Promise.all( - validatedPlugins.map((validatedPlugin) => - copyValidatedPlugin(validatedPlugin, workspacePath, config.clients, dryRun), - ), + validatedPlugins.map((validatedPlugin) => { + const skillNameMap = pluginSkillMaps.get(validatedPlugin.resolved); + return copyValidatedPlugin( + validatedPlugin, + workspacePath, + config.clients, + dryRun, + skillNameMap, + ); + }), ); // Step 5: Copy workspace files if configured diff --git a/src/core/transform.ts b/src/core/transform.ts index 09797b05..2002f15f 100644 --- a/src/core/transform.ts +++ b/src/core/transform.ts @@ -70,6 +70,19 @@ export interface CopyOptions { dryRun?: boolean; } +/** + * Options for skill copy operations + */ +export interface SkillCopyOptions extends CopyOptions { + /** + * Map of skill folder name to resolved name. + * When provided, skills will be copied using the resolved name instead of folder name. + * Key format: "folderName" (just the skill folder name) + * Value: resolved name to use for the destination + */ + skillNameMap?: Map; +} + /** * Options for workspace file copy operations */ @@ -150,16 +163,16 @@ export async function copyCommands( * @param pluginPath - Path to plugin directory * @param workspacePath - Path to workspace directory * @param client - Target client type - * @param options - Copy options (dryRun) + * @param options - Copy options (dryRun, skillNameMap) * @returns Array of copy results */ export async function copySkills( pluginPath: string, workspacePath: string, client: ClientType, - options: CopyOptions = {}, + options: SkillCopyOptions = {}, ): Promise { - const { dryRun = false } = options; + const { dryRun = false, skillNameMap } = options; const mapping = CLIENT_MAPPINGS[client]; const results: CopyResult[] = []; @@ -184,7 +197,9 @@ export async function copySkills( // Process skill directories in parallel for better performance const copyPromises = skillDirs.map(async (entry): Promise => { const skillSourcePath = join(sourceDir, entry.name); - const skillDestPath = join(destDir, entry.name); + // Use resolved name from skillNameMap if available, otherwise use folder name + const resolvedName = skillNameMap?.get(entry.name) ?? entry.name; + const skillDestPath = join(destDir, resolvedName); // Validate skill before copying const validation = await validateSkill(skillSourcePath); @@ -225,6 +240,48 @@ export async function copySkills( return Promise.all(copyPromises); } +/** + * Information about a skill collected from a plugin + */ +export interface CollectedSkill { + /** Skill folder name */ + folderName: string; + /** Path to the skill directory */ + skillPath: string; + /** Plugin path this skill belongs to */ + pluginPath: string; + /** Plugin source (original reference, e.g., GitHub URL or local path) */ + pluginSource: string; +} + +/** + * Collect skill information from a plugin without copying + * Used for the first pass of two-pass name resolution + * @param pluginPath - Resolved path to plugin directory + * @param pluginSource - Original plugin source reference + * @returns Array of collected skill information + */ +export async function collectPluginSkills( + pluginPath: string, + pluginSource: string, +): Promise { + const skillsDir = join(pluginPath, 'skills'); + + if (!existsSync(skillsDir)) { + return []; + } + + const entries = await readdir(skillsDir, { withFileTypes: true }); + const skillDirs = entries.filter((e) => e.isDirectory()); + + return skillDirs.map((entry) => ({ + folderName: entry.name, + skillPath: join(skillsDir, entry.name), + pluginPath, + pluginSource, + })); +} + /** * Copy hooks from plugin to workspace for a specific client * Only copies if client supports hooks @@ -341,27 +398,40 @@ export async function copyAgents( return Promise.all(copyPromises); } +/** + * Options for copying a plugin to workspace + */ +export interface PluginCopyOptions extends CopyOptions { + /** + * Map of skill folder name to resolved name for this specific plugin. + * When provided, skills will be copied using the resolved name instead of folder name. + */ + skillNameMap?: Map; +} + /** * Copy all plugin content to workspace for a specific client * Plugins provide: commands, skills, hooks, agents * @param pluginPath - Path to plugin directory * @param workspacePath - Path to workspace directory * @param client - Target client type - * @param options - Copy options (dryRun) + * @param options - Copy options (dryRun, skillNameMap) * @returns All copy results */ export async function copyPluginToWorkspace( pluginPath: string, workspacePath: string, client: ClientType, - options: CopyOptions = {}, + options: PluginCopyOptions = {}, ): Promise { + const { skillNameMap, ...baseOptions } = options; + // Run copy operations in parallel for better performance const [commandResults, skillResults, hookResults, agentResults] = await Promise.all([ - copyCommands(pluginPath, workspacePath, client, options), - copySkills(pluginPath, workspacePath, client, options), - copyHooks(pluginPath, workspacePath, client, options), - copyAgents(pluginPath, workspacePath, client, options), + copyCommands(pluginPath, workspacePath, client, baseOptions), + copySkills(pluginPath, workspacePath, client, { ...baseOptions, skillNameMap }), + copyHooks(pluginPath, workspacePath, client, baseOptions), + copyAgents(pluginPath, workspacePath, client, baseOptions), ]); return [...commandResults, ...skillResults, ...hookResults, ...agentResults]; diff --git a/tests/unit/core/skill-resolution.test.ts b/tests/unit/core/skill-resolution.test.ts new file mode 100644 index 00000000..e988efbd --- /dev/null +++ b/tests/unit/core/skill-resolution.test.ts @@ -0,0 +1,372 @@ +import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; +import { mkdtemp, rm, mkdir, writeFile, readdir } from 'node:fs/promises'; +import { existsSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { syncWorkspace } from '../../../src/core/sync.js'; +import { collectPluginSkills } from '../../../src/core/transform.js'; +import { CONFIG_DIR, WORKSPACE_CONFIG_FILE } from '../../../src/constants.js'; + +describe('collectPluginSkills', () => { + let testDir: string; + + beforeEach(async () => { + testDir = await mkdtemp(join(tmpdir(), 'allagents-skill-collect-')); + }); + + afterEach(async () => { + await rm(testDir, { recursive: true, force: true }); + }); + + it('should collect skills from a plugin directory', async () => { + // Setup: Create a plugin with skills + const pluginDir = join(testDir, 'my-plugin'); + await mkdir(join(pluginDir, 'skills', 'skill-a'), { recursive: true }); + await mkdir(join(pluginDir, 'skills', 'skill-b'), { recursive: true }); + await writeFile( + join(pluginDir, 'skills', 'skill-a', 'SKILL.md'), + `--- +name: skill-a +description: Skill A +--- + +# Skill A`, + ); + await writeFile( + join(pluginDir, 'skills', 'skill-b', 'SKILL.md'), + `--- +name: skill-b +description: Skill B +--- + +# Skill B`, + ); + + const skills = await collectPluginSkills(pluginDir, 'test-source'); + + expect(skills).toHaveLength(2); + expect(skills.map((s) => s.folderName).sort()).toEqual(['skill-a', 'skill-b']); + expect(skills[0]?.pluginPath).toBe(pluginDir); + expect(skills[0]?.pluginSource).toBe('test-source'); + }); + + it('should return empty array when no skills directory', async () => { + const pluginDir = join(testDir, 'empty-plugin'); + await mkdir(pluginDir, { recursive: true }); + + const skills = await collectPluginSkills(pluginDir, 'test-source'); + + expect(skills).toHaveLength(0); + }); + + it('should ignore files in skills directory (only dirs)', async () => { + const pluginDir = join(testDir, 'plugin-with-files'); + await mkdir(join(pluginDir, 'skills', 'real-skill'), { recursive: true }); + await writeFile(join(pluginDir, 'skills', 'README.md'), '# Skills'); + await writeFile( + join(pluginDir, 'skills', 'real-skill', 'SKILL.md'), + `--- +name: real-skill +description: Real Skill +---`, + ); + + const skills = await collectPluginSkills(pluginDir, 'test-source'); + + expect(skills).toHaveLength(1); + expect(skills[0]?.folderName).toBe('real-skill'); + }); +}); + +describe('skill name resolution in sync', () => { + let testDir: string; + + beforeEach(async () => { + testDir = await mkdtemp(join(tmpdir(), 'allagents-skill-sync-')); + }); + + afterEach(async () => { + await rm(testDir, { recursive: true, force: true }); + }); + + it('should use folder names when no conflicts', async () => { + // Setup: Create a plugin with a skill + const pluginDir = join(testDir, 'my-plugin'); + const skillDir = join(pluginDir, 'skills', 'unique-skill'); + await mkdir(skillDir, { recursive: true }); + await writeFile( + join(skillDir, 'SKILL.md'), + `--- +name: unique-skill +description: A unique skill +--- + +# Unique Skill`, + ); + + // Setup: Create workspace config + await mkdir(join(testDir, CONFIG_DIR), { recursive: true }); + await writeFile( + join(testDir, CONFIG_DIR, WORKSPACE_CONFIG_FILE), + ` +repositories: [] +plugins: + - ./my-plugin +clients: + - claude +`, + ); + + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + // Skill should be copied with original folder name + expect(existsSync(join(testDir, '.claude', 'skills', 'unique-skill'))).toBe(true); + }); + + it('should qualify skill names when folder names conflict across plugins', async () => { + // Setup: Create two plugins with same skill folder name but different plugin names + const plugin1Dir = join(testDir, 'plugin-alpha'); + const plugin2Dir = join(testDir, 'plugin-beta'); + + // Plugin 1 with plugin.json + await mkdir(join(plugin1Dir, 'skills', 'common-skill'), { recursive: true }); + await writeFile( + join(plugin1Dir, 'plugin.json'), + JSON.stringify({ name: 'plugin-alpha', version: '1.0.0', description: 'Plugin Alpha' }), + ); + await writeFile( + join(plugin1Dir, 'skills', 'common-skill', 'SKILL.md'), + `--- +name: common-skill +description: Common skill from alpha +--- + +# Alpha's Common Skill`, + ); + + // Plugin 2 with plugin.json + await mkdir(join(plugin2Dir, 'skills', 'common-skill'), { recursive: true }); + await writeFile( + join(plugin2Dir, 'plugin.json'), + JSON.stringify({ name: 'plugin-beta', version: '1.0.0', description: 'Plugin Beta' }), + ); + await writeFile( + join(plugin2Dir, 'skills', 'common-skill', 'SKILL.md'), + `--- +name: common-skill +description: Common skill from beta +--- + +# Beta's Common Skill`, + ); + + // Setup: Create workspace config + await mkdir(join(testDir, CONFIG_DIR), { recursive: true }); + await writeFile( + join(testDir, CONFIG_DIR, WORKSPACE_CONFIG_FILE), + ` +repositories: [] +plugins: + - ./plugin-alpha + - ./plugin-beta +clients: + - claude +`, + ); + + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + // Skills should be qualified with plugin name + const skillsDir = join(testDir, '.claude', 'skills'); + const skills = await readdir(skillsDir); + + expect(skills).toContain('plugin-alpha_common-skill'); + expect(skills).toContain('plugin-beta_common-skill'); + }); + + it('should mix renamed and non-renamed skills', async () => { + // Setup: Create two plugins - one with unique skill, one with common skill + const plugin1Dir = join(testDir, 'plugin-one'); + const plugin2Dir = join(testDir, 'plugin-two'); + + // Plugin 1 with unique and common skill + await mkdir(join(plugin1Dir, 'skills', 'unique-to-one'), { recursive: true }); + await mkdir(join(plugin1Dir, 'skills', 'shared'), { recursive: true }); + await writeFile( + join(plugin1Dir, 'plugin.json'), + JSON.stringify({ name: 'plugin-one', version: '1.0.0', description: 'Plugin One' }), + ); + await writeFile( + join(plugin1Dir, 'skills', 'unique-to-one', 'SKILL.md'), + `--- +name: unique-to-one +description: Unique skill +---`, + ); + await writeFile( + join(plugin1Dir, 'skills', 'shared', 'SKILL.md'), + `--- +name: shared +description: Shared skill from one +---`, + ); + + // Plugin 2 with common skill only + await mkdir(join(plugin2Dir, 'skills', 'shared'), { recursive: true }); + await writeFile( + join(plugin2Dir, 'plugin.json'), + JSON.stringify({ name: 'plugin-two', version: '1.0.0', description: 'Plugin Two' }), + ); + await writeFile( + join(plugin2Dir, 'skills', 'shared', 'SKILL.md'), + `--- +name: shared +description: Shared skill from two +---`, + ); + + // Setup: Create workspace config + await mkdir(join(testDir, CONFIG_DIR), { recursive: true }); + await writeFile( + join(testDir, CONFIG_DIR, WORKSPACE_CONFIG_FILE), + ` +repositories: [] +plugins: + - ./plugin-one + - ./plugin-two +clients: + - claude +`, + ); + + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + const skillsDir = join(testDir, '.claude', 'skills'); + const skills = await readdir(skillsDir); + + // Unique skill should keep original name + expect(skills).toContain('unique-to-one'); + + // Shared skills should be qualified + expect(skills).toContain('plugin-one_shared'); + expect(skills).toContain('plugin-two_shared'); + }); + + it('should track renamed skills in sync state', async () => { + // Setup: Create a plugin with skill + const pluginDir = join(testDir, 'my-plugin'); + const skillDir = join(pluginDir, 'skills', 'my-skill'); + await mkdir(skillDir, { recursive: true }); + await writeFile( + join(skillDir, 'SKILL.md'), + `--- +name: my-skill +description: A skill +---`, + ); + + // Setup: Create workspace config + await mkdir(join(testDir, CONFIG_DIR), { recursive: true }); + await writeFile( + join(testDir, CONFIG_DIR, WORKSPACE_CONFIG_FILE), + ` +repositories: [] +plugins: + - ./my-plugin +clients: + - claude +`, + ); + + // First sync + await syncWorkspace(testDir); + + // Verify state file has correct skill path + const { readFile } = await import('node:fs/promises'); + const statePath = join(testDir, CONFIG_DIR, 'sync-state.json'); + const stateContent = await readFile(statePath, 'utf-8'); + const state = JSON.parse(stateContent); + + expect(state.files.claude).toContain('.claude/skills/my-skill/'); + }); + + it('should correctly purge renamed skills on second sync', async () => { + // Setup: Create two plugins with conflicting skills + const plugin1Dir = join(testDir, 'plugin-alpha'); + const plugin2Dir = join(testDir, 'plugin-beta'); + + await mkdir(join(plugin1Dir, 'skills', 'common'), { recursive: true }); + await mkdir(join(plugin2Dir, 'skills', 'common'), { recursive: true }); + await writeFile( + join(plugin1Dir, 'plugin.json'), + JSON.stringify({ name: 'plugin-alpha', version: '1.0.0', description: 'Alpha' }), + ); + await writeFile( + join(plugin2Dir, 'plugin.json'), + JSON.stringify({ name: 'plugin-beta', version: '1.0.0', description: 'Beta' }), + ); + await writeFile( + join(plugin1Dir, 'skills', 'common', 'SKILL.md'), + `--- +name: common +description: Common skill +---`, + ); + await writeFile( + join(plugin2Dir, 'skills', 'common', 'SKILL.md'), + `--- +name: common +description: Common skill +---`, + ); + + // Setup: Create workspace config with both plugins + await mkdir(join(testDir, CONFIG_DIR), { recursive: true }); + await writeFile( + join(testDir, CONFIG_DIR, WORKSPACE_CONFIG_FILE), + ` +repositories: [] +plugins: + - ./plugin-alpha + - ./plugin-beta +clients: + - claude +`, + ); + + // First sync + await syncWorkspace(testDir); + + // Verify both qualified skills exist + expect(existsSync(join(testDir, '.claude', 'skills', 'plugin-alpha_common'))).toBe(true); + expect(existsSync(join(testDir, '.claude', 'skills', 'plugin-beta_common'))).toBe(true); + + // Remove one plugin + await writeFile( + join(testDir, CONFIG_DIR, WORKSPACE_CONFIG_FILE), + ` +repositories: [] +plugins: + - ./plugin-alpha +clients: + - claude +`, + ); + + // Second sync + await syncWorkspace(testDir); + + // Now only one plugin, so skill should revert to original name + const skillsDir = join(testDir, '.claude', 'skills'); + const skills = await readdir(skillsDir); + + // Beta's skill should be gone + expect(skills).not.toContain('plugin-beta_common'); + // Alpha's skill should now be unqualified (no conflict) + expect(skills).toContain('common'); + expect(skills).not.toContain('plugin-alpha_common'); + }); +}); From eacb5e3c1091dc01f4cee763bc58a55f561fc639 Mon Sep 17 00:00:00 2001 From: Christopher Date: Wed, 28 Jan 2026 11:52:47 +1100 Subject: [PATCH 5/9] test(sync): add integration tests for duplicate skill handling Add comprehensive integration tests that verify the full sync flow with duplicate skills across multiple scenarios: - No conflict: skill names unchanged - Skill conflict with different plugins: {plugin}_{skill} format - Skill + plugin conflict with local sources: {hash}_{plugin}_{skill} format - Mixed conflict levels in single workspace - State tracking and purging of renamed skills - Skill content preservation during renaming - Edge cases (no plugin.json, empty skills dir, etc.) Tests use the existing test infrastructure with real file system operations and verify actual sync output. Co-Authored-By: Claude Opus 4.5 --- .../2026-01-28-duplicate-skill-naming.md | 82 +++ src/core/sync.ts | 7 +- src/core/transform.ts | 2 +- src/utils/source-parser.ts | 2 +- .../skill-duplicate-github.test.ts | 456 ++++++++++++++ .../skill-duplicate-handling.test.ts | 589 ++++++++++++++++++ 6 files changed, 1133 insertions(+), 5 deletions(-) create mode 100644 .claude/plans/2026-01-28-duplicate-skill-naming.md create mode 100644 tests/integration/skill-duplicate-github.test.ts create mode 100644 tests/integration/skill-duplicate-handling.test.ts diff --git a/.claude/plans/2026-01-28-duplicate-skill-naming.md b/.claude/plans/2026-01-28-duplicate-skill-naming.md new file mode 100644 index 00000000..bf1f5f20 --- /dev/null +++ b/.claude/plans/2026-01-28-duplicate-skill-naming.md @@ -0,0 +1,82 @@ +# Duplicate Skill Name Handling + +## Problem + +When two plugins define skills with the same folder name, one overwrites the other during sync. There's no namespace or qualifier system to prevent collisions. + +## Naming Rules + +1. **No conflict** - Use skill folder name as-is + - Example: `coding-standards` + +2. **Skill folder name conflicts across plugins** - Qualify with plugin name using underscore separator + - Example: `foo_coding-standards` and `bar_coding-standards` + +3. **Both plugin name AND skill folder name conflict** - Add org/UUID prefix + - GitHub source: `anthropic_my-plugin_coding-standards` + - Local source: `a1b2c3_my-plugin_coding-standards` (6-char deterministic hash) + +## Detection Logic + +During sync, collect all skills from all plugins first, then: +1. Group skills by folder name +2. For groups with >1 skill, check if plugin names are unique +3. If plugin names also conflict, use org/UUID as final disambiguator + +## Implementation Approach + +### Where to change + +`src/core/transform.ts` in the `copySkills()` function (lines 156-226) + +### New logic flow + +1. **First pass** - Collect all skills from all plugins: + ``` + Map> + ``` + +2. **Build qualified names** - For each skill: + - If only one skill with that folder name β†’ use folder name as-is + - If multiple skills share folder name but different plugin names β†’ `{plugin}_{skill}` + - If multiple skills share both β†’ `{orgOrUuid}_{plugin}_{skill}` + +3. **Second pass** - Copy skills using the resolved names + +### Deriving org/UUID + +- GitHub URL source (e.g., `github:anthropic/superpowers`) β†’ extract org (`anthropic`) +- Local path source β†’ generate deterministic 6-char hash from the absolute path + +```typescript +import { createHash } from 'crypto'; + +function getShortId(localPath: string): string { + return createHash('sha256') + .update(localPath) + .digest('hex') + .substring(0, 6); +} +``` + +## Files to Modify + +| File | Change | +|------|--------| +| `src/core/transform.ts` | Refactor `copySkills()` to two-pass with name resolution | +| `src/utils/hash.ts` (new) | Small utility for 6-char deterministic hash | +| `src/utils/source-parser.ts` (new or existing) | Extract org from GitHub URLs | + +## Edge Cases + +- Plugin source URL parsing: `github:anthropic/repo` vs `github:anthropic/repo#branch` vs local paths +- Skill folder names that already contain underscores (e.g., `my_skill` from plugin `foo` β†’ `foo_my_skill`) +- Empty or missing org in GitHub URL (fallback to hash) + +## Tests to Add + +- No conflict β†’ skill name unchanged +- Skill conflict with different plugins β†’ `plugin_skill` format +- Skill + plugin conflict with GitHub sources β†’ `org_plugin_skill` format +- Skill + plugin conflict with local sources β†’ `hash_plugin_skill` format +- Mixed sources (some GitHub, some local) with conflicts diff --git a/src/core/sync.ts b/src/core/sync.ts index 3e43095c..599b8db8 100644 --- a/src/core/sync.ts +++ b/src/core/sync.ts @@ -686,7 +686,7 @@ async function copyValidatedPlugin( validatedPlugin.resolved, workspacePath, client as ClientType, - { dryRun, skillNameMap }, + { dryRun, ...(skillNameMap && { skillNameMap }) }, ); copyResults.push(...results); } @@ -765,8 +765,9 @@ function buildPluginSkillNameMaps( const pluginMaps = new Map>(); for (let i = 0; i < allSkills.length; i++) { - const skill = allSkills[i]!; - const entry = skillEntries[i]!; + const skill = allSkills[i]; + const entry = skillEntries[i]; + if (!skill || !entry) continue; const resolvedName = resolution.nameMap.get(getSkillKey(entry)); if (resolvedName) { diff --git a/src/core/transform.ts b/src/core/transform.ts index 2002f15f..eae0e234 100644 --- a/src/core/transform.ts +++ b/src/core/transform.ts @@ -429,7 +429,7 @@ export async function copyPluginToWorkspace( // Run copy operations in parallel for better performance const [commandResults, skillResults, hookResults, agentResults] = await Promise.all([ copyCommands(pluginPath, workspacePath, client, baseOptions), - copySkills(pluginPath, workspacePath, client, { ...baseOptions, skillNameMap }), + copySkills(pluginPath, workspacePath, client, { ...baseOptions, ...(skillNameMap && { skillNameMap }) }), copyHooks(pluginPath, workspacePath, client, baseOptions), copyAgents(pluginPath, workspacePath, client, baseOptions), ]); diff --git a/src/utils/source-parser.ts b/src/utils/source-parser.ts index dd200c45..7b9be1a8 100644 --- a/src/utils/source-parser.ts +++ b/src/utils/source-parser.ts @@ -52,7 +52,7 @@ export function extractOrgFromSource(source: string): string | null { const urlMatch = trimmedSource.match( /^(?:https?:\/\/)?(?:www\.)?github\.com\/([^/]+)(?:\/|$)/ ); - if (urlMatch && urlMatch[1]) { + if (urlMatch?.[1]) { return validateOrg(urlMatch[1]); } diff --git a/tests/integration/skill-duplicate-github.test.ts b/tests/integration/skill-duplicate-github.test.ts new file mode 100644 index 00000000..09b4e244 --- /dev/null +++ b/tests/integration/skill-duplicate-github.test.ts @@ -0,0 +1,456 @@ +/** + * Integration tests for duplicate skill handling with GitHub sources. + * + * These tests verify the org_plugin_skill naming format when plugins come from + * GitHub sources. We mock the execa module to simulate successful GitHub fetches + * pointing to local test directories. + * + * Test scenarios: + * 1. Skill + plugin conflict with GitHub sources - {org}_{plugin}_{skill} format + * 2. Mixed GitHub + local sources with conflicts + */ + +import { describe, it, expect, beforeEach, afterEach, mock } from 'bun:test'; +import { mkdtemp, rm, mkdir, writeFile, readdir, readFile } from 'node:fs/promises'; +import { existsSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { CONFIG_DIR, WORKSPACE_CONFIG_FILE } from '../../src/constants.js'; +import { getShortId } from '../../src/utils/hash.js'; + +// Test directory references +let testDir: string; +let cacheBaseDir: string; + +// Track which GitHub paths map to which local test paths +const githubToLocalMap = new Map(); + +// Override HOME to use test directory for cache +const originalHome = process.env.HOME; + +// Mock execa to intercept gh commands +const execaMock = mock(async (cmd: string, args: string[]) => { + if (cmd === 'gh') { + if (args[0] === '--version') { + return { stdout: 'gh version 2.40.0', stderr: '' }; + } + if (args[0] === 'repo' && args[1] === 'clone') { + // args[2] is the repo (owner/repo) + // args[3] is the destination path + const repo = args[2]; + const destPath = args[3]; + + // Find the local source for this repo + const localSource = githubToLocalMap.get(repo); + if (localSource) { + // Copy the local plugin to the cache path + await copyDir(localSource, destPath); + return { stdout: '', stderr: '' }; + } + throw new Error(`Unknown repo: ${repo}`); + } + } + throw new Error(`Unexpected command: ${cmd} ${args.join(' ')}`); +}); + +// Helper to recursively copy a directory +async function copyDir(src: string, dest: string): Promise { + await mkdir(dest, { recursive: true }); + const entries = await readdir(src, { withFileTypes: true }); + for (const entry of entries) { + const srcPath = join(src, entry.name); + const destPath = join(dest, entry.name); + if (entry.isDirectory()) { + await copyDir(srcPath, destPath); + } else { + const content = await readFile(srcPath); + await writeFile(destPath, content); + } + } +} + +// Mock the execa module BEFORE importing sync +mock.module('execa', () => ({ + execa: execaMock, +})); + +// Now import syncWorkspace after mocking +const { syncWorkspace } = await import('../../src/core/sync.js'); + +describe('Skill duplicate handling with GitHub sources', () => { + beforeEach(async () => { + testDir = await mkdtemp(join(tmpdir(), 'allagents-github-skill-')); + cacheBaseDir = join(testDir, 'home'); + + // Set HOME to our test directory so cache goes there + process.env.HOME = cacheBaseDir; + + // Clear the map and mocks + githubToLocalMap.clear(); + execaMock.mockClear(); + }); + + afterEach(async () => { + // Restore HOME + process.env.HOME = originalHome; + await rm(testDir, { recursive: true, force: true }); + }); + + /** + * Helper to create a plugin with skills + */ + async function createPlugin( + pluginPath: string, + pluginName: string, + skillNames: string[], + ): Promise { + await mkdir(pluginPath, { recursive: true }); + + // Create plugin.json + await writeFile( + join(pluginPath, 'plugin.json'), + JSON.stringify({ + name: pluginName, + version: '1.0.0', + description: `Test plugin ${pluginName}`, + }), + ); + + // Create skills + for (const skillName of skillNames) { + const skillDir = join(pluginPath, 'skills', skillName); + await mkdir(skillDir, { recursive: true }); + await writeFile( + join(skillDir, 'SKILL.md'), + `--- +name: ${skillName} +description: Skill ${skillName} from ${pluginName} +--- + +# ${skillName} + +This skill is from ${pluginName}. +`, + ); + } + } + + /** + * Helper to create workspace.yaml with specific plugin sources + */ + async function createWorkspaceConfig(plugins: string[]): Promise { + await mkdir(join(testDir, CONFIG_DIR), { recursive: true }); + await writeFile( + join(testDir, CONFIG_DIR, WORKSPACE_CONFIG_FILE), + ` +repositories: [] +plugins: +${plugins.map((p) => ` - ${p}`).join('\n')} +clients: + - claude +`, + ); + } + + /** + * Helper to get all skill directory names after sync + */ + async function getSyncedSkills(): Promise { + const skillsDir = join(testDir, '.claude', 'skills'); + if (!existsSync(skillsDir)) { + return []; + } + return readdir(skillsDir); + } + + /** + * Helper to register a GitHub source with its local cache path + * Uses gh: prefix which is recognized by both isGitHubUrl and extractOrgFromSource + */ + function registerGitHubSource(org: string, repo: string, localPath: string): string { + const key = `${org}/${repo}`; + githubToLocalMap.set(key, localPath); + return `gh:${key}`; + } + + describe('Scenario: Skill + plugin conflict with GitHub sources - {org}_{plugin}_{skill} format', () => { + it('should use org prefix when same plugin name exists in different GitHub orgs', async () => { + // Setup: Two plugins with same name from different GitHub orgs + const path1 = join(testDir, 'source', 'acme-corp', 'tools'); + const path2 = join(testDir, 'source', 'beta-inc', 'tools'); + + await createPlugin(path1, 'tools', ['deploy']); + await createPlugin(path2, 'tools', ['deploy']); + + // Register GitHub sources + const source1 = registerGitHubSource('acme-corp', 'plugins', path1); + const source2 = registerGitHubSource('beta-inc', 'plugins', path2); + + await createWorkspaceConfig([source1, source2]); + + // Sync + const result = await syncWorkspace(testDir); + + if (!result.success) { + console.error('Sync failed:', result.error); + console.error('Plugin results:', JSON.stringify(result.pluginResults, null, 2)); + } + expect(result.success).toBe(true); + + // Verify: Skills should be prefixed with org name + // Format: {org}_{plugin}_{skill} + const skills = await getSyncedSkills(); + expect(skills).toHaveLength(2); + expect(skills).toContain('acme-corp_tools_deploy'); + expect(skills).toContain('beta-inc_tools_deploy'); + }); + + it('should handle three-way plugin name conflict with GitHub orgs', async () => { + // Setup: Three plugins with same name from different orgs + const path1 = join(testDir, 'source', 'org-alpha', 'shared'); + const path2 = join(testDir, 'source', 'org-beta', 'shared'); + const path3 = join(testDir, 'source', 'org-gamma', 'shared'); + + await createPlugin(path1, 'shared-plugin', ['common-skill']); + await createPlugin(path2, 'shared-plugin', ['common-skill']); + await createPlugin(path3, 'shared-plugin', ['common-skill']); + + const source1 = registerGitHubSource('org-alpha', 'plugins', path1); + const source2 = registerGitHubSource('org-beta', 'plugins', path2); + const source3 = registerGitHubSource('org-gamma', 'plugins', path3); + + await createWorkspaceConfig([source1, source2, source3]); + + // Sync + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + // Verify: All three should have org prefix + const skills = await getSyncedSkills(); + expect(skills).toHaveLength(3); + expect(skills).toContain('org-alpha_shared-plugin_common-skill'); + expect(skills).toContain('org-beta_shared-plugin_common-skill'); + expect(skills).toContain('org-gamma_shared-plugin_common-skill'); + }); + + it('should preserve skill content when using org prefix', async () => { + // Setup: Two plugins with same name, different content + const path1 = join(testDir, 'source', 'company-a', 'my-plugin'); + const path2 = join(testDir, 'source', 'company-b', 'my-plugin'); + + await mkdir(join(path1, 'skills', 'coding'), { recursive: true }); + await mkdir(join(path2, 'skills', 'coding'), { recursive: true }); + + await writeFile( + join(path1, 'plugin.json'), + JSON.stringify({ name: 'my-plugin', version: '1.0.0', description: 'Company A plugin' }), + ); + await writeFile( + join(path2, 'plugin.json'), + JSON.stringify({ name: 'my-plugin', version: '1.0.0', description: 'Company B plugin' }), + ); + + await writeFile( + join(path1, 'skills', 'coding', 'SKILL.md'), + `--- +name: coding +description: Company A coding +--- +# Coding +Company A implementation. +`, + ); + + await writeFile( + join(path2, 'skills', 'coding', 'SKILL.md'), + `--- +name: coding +description: Company B coding +--- +# Coding +Company B implementation. +`, + ); + + const source1 = registerGitHubSource('company-a', 'plugins', path1); + const source2 = registerGitHubSource('company-b', 'plugins', path2); + + await createWorkspaceConfig([source1, source2]); + + // Sync + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + // Verify content is correct + const contentA = await readFile( + join(testDir, '.claude', 'skills', 'company-a_my-plugin_coding', 'SKILL.md'), + 'utf-8', + ); + const contentB = await readFile( + join(testDir, '.claude', 'skills', 'company-b_my-plugin_coding', 'SKILL.md'), + 'utf-8', + ); + + expect(contentA).toContain('Company A implementation'); + expect(contentB).toContain('Company B implementation'); + }); + }); + + describe('Scenario: Mixed GitHub + local sources with conflicts', () => { + it('should use org prefix for GitHub and hash prefix for local sources', async () => { + // Setup: One GitHub plugin and one local plugin with same name + const githubPath = join(testDir, 'source', 'acme', 'my-plugin'); + const localPath = join(testDir, 'local', 'my-plugin'); + + await createPlugin(githubPath, 'my-plugin', ['build']); + await createPlugin(localPath, 'my-plugin', ['build']); + + const githubSource = registerGitHubSource('acme', 'plugins', githubPath); + const localSource = './local/my-plugin'; + + await createWorkspaceConfig([githubSource, localSource]); + + // Sync + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + // Verify: GitHub source uses org prefix, local uses hash prefix + const skills = await getSyncedSkills(); + expect(skills).toHaveLength(2); + + // GitHub source should have org prefix + expect(skills).toContain('acme_my-plugin_build'); + + // Local source should have hash prefix + const localHash = getShortId(localSource); + expect(skills).toContain(`${localHash}_my-plugin_build`); + }); + + it('should handle mixed sources where only some conflict', async () => { + // Setup: + // - GitHub plugin from acme: has "shared" and "unique-github" skills + // - Local plugin: has "shared" and "unique-local" skills + // - Another GitHub plugin from beta: has "other" skill (no conflict) + + const acmePath = join(testDir, 'source', 'acme', 'devtools'); + const localPath = join(testDir, 'local', 'devtools'); + const betaPath = join(testDir, 'source', 'beta', 'utilities'); + + await createPlugin(acmePath, 'devtools', ['shared', 'unique-github']); + await createPlugin(localPath, 'devtools', ['shared', 'unique-local']); + await createPlugin(betaPath, 'utilities', ['other']); + + const acmeSource = registerGitHubSource('acme', 'devtools', acmePath); + const betaSource = registerGitHubSource('beta', 'utilities', betaPath); + const localSource = './local/devtools'; + + await createWorkspaceConfig([acmeSource, localSource, betaSource]); + + // Sync + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + const skills = await getSyncedSkills(); + + // Unique skills should keep original names + expect(skills).toContain('unique-github'); + expect(skills).toContain('unique-local'); + expect(skills).toContain('other'); + + // Conflicting skills should be fully qualified + // Both have same plugin name "devtools" and same skill "shared" + const localHash = getShortId(localSource); + expect(skills).toContain('acme_devtools_shared'); + expect(skills).toContain(`${localHash}_devtools_shared`); + }); + + it('should handle multiple GitHub orgs and multiple local paths all conflicting', async () => { + // Setup: Four plugins all with same name and same skill + const acmePath = join(testDir, 'source', 'acme', 'builder'); + const betaPath = join(testDir, 'source', 'beta', 'builder'); + const local1Path = join(testDir, 'vendor', 'builder'); + const local2Path = join(testDir, 'custom', 'builder'); + + await createPlugin(acmePath, 'builder', ['compile']); + await createPlugin(betaPath, 'builder', ['compile']); + await createPlugin(local1Path, 'builder', ['compile']); + await createPlugin(local2Path, 'builder', ['compile']); + + const acmeSource = registerGitHubSource('acme', 'builder', acmePath); + const betaSource = registerGitHubSource('beta', 'builder', betaPath); + const local1Source = './vendor/builder'; + const local2Source = './custom/builder'; + + await createWorkspaceConfig([acmeSource, betaSource, local1Source, local2Source]); + + // Sync + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + const skills = await getSyncedSkills(); + expect(skills).toHaveLength(4); + + // GitHub sources use org prefix + expect(skills).toContain('acme_builder_compile'); + expect(skills).toContain('beta_builder_compile'); + + // Local sources use hash prefix + const hash1 = getShortId(local1Source); + const hash2 = getShortId(local2Source); + expect(skills).toContain(`${hash1}_builder_compile`); + expect(skills).toContain(`${hash2}_builder_compile`); + }); + + it('should track renamed skills correctly in sync state for mixed sources', async () => { + // Setup: One GitHub and one local with same plugin name + const githubPath = join(testDir, 'source', 'myorg', 'analyzer'); + const localPath = join(testDir, 'local', 'analyzer'); + + await createPlugin(githubPath, 'analyzer', ['lint']); + await createPlugin(localPath, 'analyzer', ['lint']); + + const githubSource = registerGitHubSource('myorg', 'analyzer', githubPath); + const localSource = './local/analyzer'; + + await createWorkspaceConfig([githubSource, localSource]); + + // Sync + await syncWorkspace(testDir); + + // Verify state file contains renamed skill paths + const statePath = join(testDir, CONFIG_DIR, 'sync-state.json'); + const stateContent = await readFile(statePath, 'utf-8'); + const state = JSON.parse(stateContent); + + const localHash = getShortId(localSource); + + // Both renamed paths should be in state + expect(state.files.claude).toContain('.claude/skills/myorg_analyzer_lint/'); + expect(state.files.claude).toContain(`.claude/skills/${localHash}_analyzer_lint/`); + }); + }); + + describe('Scenario: GitHub shorthand formats', () => { + it('should extract org from gh: shorthand format', async () => { + // Setup: Using gh: prefix (registered via helper) + const path1 = join(testDir, 'source', 'team-x', 'tools'); + const path2 = join(testDir, 'source', 'team-y', 'tools'); + + await createPlugin(path1, 'tools', ['run']); + await createPlugin(path2, 'tools', ['run']); + + const source1 = registerGitHubSource('team-x', 'tools', path1); + const source2 = registerGitHubSource('team-y', 'tools', path2); + + await createWorkspaceConfig([source1, source2]); + + // Sync + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + const skills = await getSyncedSkills(); + expect(skills).toContain('team-x_tools_run'); + expect(skills).toContain('team-y_tools_run'); + }); + }); +}); diff --git a/tests/integration/skill-duplicate-handling.test.ts b/tests/integration/skill-duplicate-handling.test.ts new file mode 100644 index 00000000..00f82a08 --- /dev/null +++ b/tests/integration/skill-duplicate-handling.test.ts @@ -0,0 +1,589 @@ +/** + * Integration tests for duplicate skill handling in the full sync flow. + * + * These tests verify that when multiple plugins have skills with the same + * folder name, the sync correctly disambiguates them based on: + * 1. No conflict - skill name unchanged + * 2. Skill conflict with different plugins - {plugin}_{skill} format + * 3. Skill + plugin conflict with GitHub sources - {org}_{plugin}_{skill} format + * 4. Skill + plugin conflict with local sources - {hash}_{plugin}_{skill} format + * 5. Mixed sources (some GitHub, some local) with conflicts + */ + +import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; +import { mkdtemp, rm, mkdir, writeFile, readdir, readFile } from 'node:fs/promises'; +import { existsSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { syncWorkspace } from '../../src/core/sync.js'; +import { CONFIG_DIR, WORKSPACE_CONFIG_FILE } from '../../src/constants.js'; +import { getShortId } from '../../src/utils/hash.js'; + +describe('Skill duplicate handling integration tests', () => { + let testDir: string; + + beforeEach(async () => { + testDir = await mkdtemp(join(tmpdir(), 'allagents-skill-dup-')); + }); + + afterEach(async () => { + await rm(testDir, { recursive: true, force: true }); + }); + + /** + * Helper to create a plugin with skills + */ + async function createPlugin( + pluginPath: string, + pluginName: string, + skillNames: string[], + ): Promise { + await mkdir(pluginPath, { recursive: true }); + + // Create plugin.json + await writeFile( + join(pluginPath, 'plugin.json'), + JSON.stringify({ + name: pluginName, + version: '1.0.0', + description: `Test plugin ${pluginName}`, + }), + ); + + // Create skills + for (const skillName of skillNames) { + const skillDir = join(pluginPath, 'skills', skillName); + await mkdir(skillDir, { recursive: true }); + await writeFile( + join(skillDir, 'SKILL.md'), + `--- +name: ${skillName} +description: Skill ${skillName} from ${pluginName} +--- + +# ${skillName} + +This skill is from ${pluginName}. +`, + ); + } + } + + /** + * Helper to create workspace.yaml + */ + async function createWorkspaceConfig(plugins: string[]): Promise { + await mkdir(join(testDir, CONFIG_DIR), { recursive: true }); + await writeFile( + join(testDir, CONFIG_DIR, WORKSPACE_CONFIG_FILE), + ` +repositories: [] +plugins: +${plugins.map((p) => ` - ${p}`).join('\n')} +clients: + - claude +`, + ); + } + + /** + * Helper to get all skill directory names after sync + */ + async function getSyncedSkills(): Promise { + const skillsDir = join(testDir, '.claude', 'skills'); + if (!existsSync(skillsDir)) { + return []; + } + return readdir(skillsDir); + } + + describe('Scenario 1: No conflict - skill name unchanged', () => { + it('should keep original folder names when no skills conflict', async () => { + // Setup: Two plugins with unique skill names + await createPlugin(join(testDir, 'plugin-alpha'), 'plugin-alpha', [ + 'skill-a', + 'skill-b', + ]); + await createPlugin(join(testDir, 'plugin-beta'), 'plugin-beta', [ + 'skill-c', + 'skill-d', + ]); + + await createWorkspaceConfig(['./plugin-alpha', './plugin-beta']); + + // Sync + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + // Verify skills keep original names + const skills = await getSyncedSkills(); + expect(skills.sort()).toEqual(['skill-a', 'skill-b', 'skill-c', 'skill-d']); + }); + + it('should handle single plugin with multiple unique skills', async () => { + // Setup: One plugin with multiple skills + await createPlugin(join(testDir, 'my-plugin'), 'my-plugin', [ + 'coding', + 'testing', + 'debugging', + ]); + + await createWorkspaceConfig(['./my-plugin']); + + // Sync + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + // Verify skills keep original names + const skills = await getSyncedSkills(); + expect(skills.sort()).toEqual(['coding', 'debugging', 'testing']); + }); + }); + + describe('Scenario 2: Skill conflict with different plugin names - {plugin}_{skill} format', () => { + it('should qualify skill names with plugin prefix when folder names conflict', async () => { + // Setup: Two plugins with same skill folder name but different plugin names + await createPlugin(join(testDir, 'alpha-tools'), 'alpha-tools', ['common-skill']); + await createPlugin(join(testDir, 'beta-tools'), 'beta-tools', ['common-skill']); + + await createWorkspaceConfig(['./alpha-tools', './beta-tools']); + + // Sync + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + // Verify skills are qualified with plugin name + const skills = await getSyncedSkills(); + expect(skills.sort()).toEqual(['alpha-tools_common-skill', 'beta-tools_common-skill']); + }); + + it('should only rename conflicting skills, not unique ones', async () => { + // Setup: Two plugins, one skill conflicts, others unique + await createPlugin(join(testDir, 'plugin-one'), 'plugin-one', [ + 'unique-to-one', + 'shared-skill', + ]); + await createPlugin(join(testDir, 'plugin-two'), 'plugin-two', [ + 'shared-skill', + 'unique-to-two', + ]); + + await createWorkspaceConfig(['./plugin-one', './plugin-two']); + + // Sync + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + // Verify: unique skills unchanged, shared skills qualified + const skills = await getSyncedSkills(); + expect(skills).toContain('unique-to-one'); + expect(skills).toContain('unique-to-two'); + expect(skills).toContain('plugin-one_shared-skill'); + expect(skills).toContain('plugin-two_shared-skill'); + expect(skills).not.toContain('shared-skill'); // Should be qualified + }); + + it('should handle multiple conflicting skills across three plugins', async () => { + // Setup: Three plugins with overlapping skills + await createPlugin(join(testDir, 'plugin-a'), 'plugin-a', ['common', 'special-a']); + await createPlugin(join(testDir, 'plugin-b'), 'plugin-b', ['common', 'special-b']); + await createPlugin(join(testDir, 'plugin-c'), 'plugin-c', ['common', 'special-c']); + + await createWorkspaceConfig(['./plugin-a', './plugin-b', './plugin-c']); + + // Sync + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + // Verify: unique skills unchanged, common skill qualified for all three + const skills = await getSyncedSkills(); + expect(skills).toContain('special-a'); + expect(skills).toContain('special-b'); + expect(skills).toContain('special-c'); + expect(skills).toContain('plugin-a_common'); + expect(skills).toContain('plugin-b_common'); + expect(skills).toContain('plugin-c_common'); + }); + }); + + describe('Scenario 3: Skill + plugin conflict with local sources - {hash}_{plugin}_{skill} format', () => { + it('should add hash prefix when both skill folder and plugin name conflict (local paths)', async () => { + // Setup: Two plugins from DIFFERENT local paths with SAME plugin name + // This simulates having two different versions/forks of the same plugin + const path1 = join(testDir, 'vendor', 'acme', 'my-plugin'); + const path2 = join(testDir, 'local', 'custom', 'my-plugin'); + + await createPlugin(path1, 'my-plugin', ['coding']); + await createPlugin(path2, 'my-plugin', ['coding']); + + // Workspace config uses relative paths from testDir + await createWorkspaceConfig(['./vendor/acme/my-plugin', './local/custom/my-plugin']); + + // Sync + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + // Verify: Skills should be prefixed with hash of the source path + const skills = await getSyncedSkills(); + expect(skills).toHaveLength(2); + + // Both skills should have the pattern: {6-char-hash}_my-plugin_coding + // The hash is based on the plugin source string + const hash1 = getShortId('./vendor/acme/my-plugin'); + const hash2 = getShortId('./local/custom/my-plugin'); + + expect(skills).toContain(`${hash1}_my-plugin_coding`); + expect(skills).toContain(`${hash2}_my-plugin_coding`); + }); + + it('should handle multiple skills with full disambiguation', async () => { + // Setup: Two plugins from different paths, same name, multiple conflicting skills + const path1 = join(testDir, 'team-a', 'shared-plugin'); + const path2 = join(testDir, 'team-b', 'shared-plugin'); + + await createPlugin(path1, 'shared-plugin', ['analyze', 'generate', 'validate']); + await createPlugin(path2, 'shared-plugin', ['analyze', 'transform', 'validate']); + + await createWorkspaceConfig(['./team-a/shared-plugin', './team-b/shared-plugin']); + + // Sync + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + const skills = await getSyncedSkills(); + + // Unique skills should keep original names + expect(skills).toContain('generate'); + expect(skills).toContain('transform'); + + // Conflicting skills should have hash prefix + const hash1 = getShortId('./team-a/shared-plugin'); + const hash2 = getShortId('./team-b/shared-plugin'); + + expect(skills).toContain(`${hash1}_shared-plugin_analyze`); + expect(skills).toContain(`${hash2}_shared-plugin_analyze`); + expect(skills).toContain(`${hash1}_shared-plugin_validate`); + expect(skills).toContain(`${hash2}_shared-plugin_validate`); + }); + }); + + describe('Scenario 4: Mixed conflict levels', () => { + it('should handle mix of no-conflict, plugin-conflict, and full-conflict skills', async () => { + // Setup: + // - plugin-unique: has "unique-skill" (no conflict) + // - plugin-alpha: has "shared-skill" (conflicts with beta, different plugin names) + // - plugin-beta: has "shared-skill" (conflicts with alpha) + // - same-name plugins from different paths with "common" skill (full conflict) + + await createPlugin(join(testDir, 'plugin-unique'), 'plugin-unique', ['unique-skill']); + await createPlugin(join(testDir, 'plugin-alpha'), 'plugin-alpha', ['shared-skill']); + await createPlugin(join(testDir, 'plugin-beta'), 'plugin-beta', ['shared-skill']); + await createPlugin(join(testDir, 'path-a', 'forked-plugin'), 'forked-plugin', ['common']); + await createPlugin(join(testDir, 'path-b', 'forked-plugin'), 'forked-plugin', ['common']); + + await createWorkspaceConfig([ + './plugin-unique', + './plugin-alpha', + './plugin-beta', + './path-a/forked-plugin', + './path-b/forked-plugin', + ]); + + // Sync + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + const skills = await getSyncedSkills(); + + // No conflict - keeps original name + expect(skills).toContain('unique-skill'); + + // Plugin-level conflict - {plugin}_{skill} + expect(skills).toContain('plugin-alpha_shared-skill'); + expect(skills).toContain('plugin-beta_shared-skill'); + + // Full conflict - {hash}_{plugin}_{skill} + const hashA = getShortId('./path-a/forked-plugin'); + const hashB = getShortId('./path-b/forked-plugin'); + expect(skills).toContain(`${hashA}_forked-plugin_common`); + expect(skills).toContain(`${hashB}_forked-plugin_common`); + }); + }); + + describe('Scenario 5: State tracking and purging of renamed skills', () => { + it('should correctly track renamed skills in sync state', async () => { + // Setup: Two plugins with conflicting skills + await createPlugin(join(testDir, 'plugin-a'), 'plugin-a', ['shared']); + await createPlugin(join(testDir, 'plugin-b'), 'plugin-b', ['shared']); + + await createWorkspaceConfig(['./plugin-a', './plugin-b']); + + // First sync + await syncWorkspace(testDir); + + // Verify state file contains renamed skill paths + const statePath = join(testDir, CONFIG_DIR, 'sync-state.json'); + const stateContent = await readFile(statePath, 'utf-8'); + const state = JSON.parse(stateContent); + + expect(state.files.claude).toContain('.claude/skills/plugin-a_shared/'); + expect(state.files.claude).toContain('.claude/skills/plugin-b_shared/'); + }); + + it('should purge renamed skills and revert to original names when conflict resolves', async () => { + // Setup: Two plugins with conflicting skills + await createPlugin(join(testDir, 'plugin-a'), 'plugin-a', ['shared']); + await createPlugin(join(testDir, 'plugin-b'), 'plugin-b', ['shared']); + + await createWorkspaceConfig(['./plugin-a', './plugin-b']); + + // First sync - should create qualified names + await syncWorkspace(testDir); + + let skills = await getSyncedSkills(); + expect(skills).toContain('plugin-a_shared'); + expect(skills).toContain('plugin-b_shared'); + + // Remove plugin-b from workspace + await createWorkspaceConfig(['./plugin-a']); + + // Second sync - conflict resolved, should use original name + await syncWorkspace(testDir); + + skills = await getSyncedSkills(); + expect(skills).toContain('shared'); // Reverted to original + expect(skills).not.toContain('plugin-a_shared'); // Old qualified name gone + expect(skills).not.toContain('plugin-b_shared'); // Removed plugin's skill gone + }); + + it('should handle transition from non-conflicting to conflicting state', async () => { + // Setup: Start with one plugin + await createPlugin(join(testDir, 'plugin-a'), 'plugin-a', ['coding']); + + await createWorkspaceConfig(['./plugin-a']); + + // First sync - no conflict + await syncWorkspace(testDir); + + let skills = await getSyncedSkills(); + expect(skills).toContain('coding'); + + // Add second plugin with same skill name + await createPlugin(join(testDir, 'plugin-b'), 'plugin-b', ['coding']); + await createWorkspaceConfig(['./plugin-a', './plugin-b']); + + // Second sync - now has conflict + await syncWorkspace(testDir); + + skills = await getSyncedSkills(); + expect(skills).not.toContain('coding'); // Original name should be gone + expect(skills).toContain('plugin-a_coding'); + expect(skills).toContain('plugin-b_coding'); + }); + }); + + describe('Scenario 6: Skill content verification', () => { + it('should preserve skill content when renaming', async () => { + // Setup: Two plugins with conflicting skill names + await createPlugin(join(testDir, 'plugin-alpha'), 'plugin-alpha', ['common']); + await createPlugin(join(testDir, 'plugin-beta'), 'plugin-beta', ['common']); + + // Modify skill content to be unique + await writeFile( + join(testDir, 'plugin-alpha', 'skills', 'common', 'SKILL.md'), + `--- +name: common +description: Alpha version +--- + +# Common Skill + +This is the ALPHA version. +`, + ); + + await writeFile( + join(testDir, 'plugin-beta', 'skills', 'common', 'SKILL.md'), + `--- +name: common +description: Beta version +--- + +# Common Skill + +This is the BETA version. +`, + ); + + await createWorkspaceConfig(['./plugin-alpha', './plugin-beta']); + + // Sync + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + // Verify content is preserved correctly + const alphaContent = await readFile( + join(testDir, '.claude', 'skills', 'plugin-alpha_common', 'SKILL.md'), + 'utf-8', + ); + const betaContent = await readFile( + join(testDir, '.claude', 'skills', 'plugin-beta_common', 'SKILL.md'), + 'utf-8', + ); + + expect(alphaContent).toContain('ALPHA version'); + expect(betaContent).toContain('BETA version'); + }); + + it('should copy all files in skill directory when renaming', async () => { + // Setup: Plugin with skill containing multiple files + const pluginDir = join(testDir, 'plugin-alpha'); + const skillDir = join(pluginDir, 'skills', 'complex-skill'); + + await mkdir(skillDir, { recursive: true }); + await writeFile( + join(pluginDir, 'plugin.json'), + JSON.stringify({ name: 'plugin-alpha', version: '1.0.0', description: 'Alpha' }), + ); + await writeFile( + join(skillDir, 'SKILL.md'), + `--- +name: complex-skill +description: A complex skill +--- + +# Complex Skill +`, + ); + await writeFile(join(skillDir, 'helper.py'), '# Helper script'); + await writeFile(join(skillDir, 'config.json'), '{"setting": true}'); + + // Second plugin with same skill name + await createPlugin(join(testDir, 'plugin-beta'), 'plugin-beta', ['complex-skill']); + + await createWorkspaceConfig(['./plugin-alpha', './plugin-beta']); + + // Sync + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + // Verify all files are copied to renamed directory + const renamedDir = join(testDir, '.claude', 'skills', 'plugin-alpha_complex-skill'); + expect(existsSync(join(renamedDir, 'SKILL.md'))).toBe(true); + expect(existsSync(join(renamedDir, 'helper.py'))).toBe(true); + expect(existsSync(join(renamedDir, 'config.json'))).toBe(true); + }); + }); + + describe('Scenario 7: Edge cases', () => { + it('should handle plugins without plugin.json (fallback to directory name)', async () => { + // Setup: Plugins without plugin.json + const plugin1Dir = join(testDir, 'tools-alpha'); + const plugin2Dir = join(testDir, 'tools-beta'); + + await mkdir(join(plugin1Dir, 'skills', 'shared'), { recursive: true }); + await mkdir(join(plugin2Dir, 'skills', 'shared'), { recursive: true }); + + // No plugin.json - should use directory name + await writeFile( + join(plugin1Dir, 'skills', 'shared', 'SKILL.md'), + `--- +name: shared +description: Shared from alpha +---`, + ); + await writeFile( + join(plugin2Dir, 'skills', 'shared', 'SKILL.md'), + `--- +name: shared +description: Shared from beta +---`, + ); + + await createWorkspaceConfig(['./tools-alpha', './tools-beta']); + + // Sync + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + // Verify skills are qualified with directory name + const skills = await getSyncedSkills(); + expect(skills).toContain('tools-alpha_shared'); + expect(skills).toContain('tools-beta_shared'); + }); + + it('should handle skill names with hyphens correctly', async () => { + // Setup: Skills with various hyphenated naming conventions + // Note: Underscores are NOT allowed in skill names per validation rules + await createPlugin(join(testDir, 'plugin-a'), 'plugin-a', [ + 'my-skill', + 'my-other-skill', + 'my-unique-skill', + ]); + await createPlugin(join(testDir, 'plugin-b'), 'plugin-b', ['my-skill', 'my-other-skill']); + + await createWorkspaceConfig(['./plugin-a', './plugin-b']); + + // Sync + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + const skills = await getSyncedSkills(); + + // Unique skill keeps original name + expect(skills).toContain('my-unique-skill'); + + // Conflicting skills are qualified + expect(skills).toContain('plugin-a_my-skill'); + expect(skills).toContain('plugin-b_my-skill'); + expect(skills).toContain('plugin-a_my-other-skill'); + expect(skills).toContain('plugin-b_my-other-skill'); + }); + + it('should handle empty skills directory gracefully', async () => { + // Setup: Plugin with skills directory but no skills + const pluginDir = join(testDir, 'empty-plugin'); + await mkdir(join(pluginDir, 'skills'), { recursive: true }); + await writeFile( + join(pluginDir, 'plugin.json'), + JSON.stringify({ name: 'empty-plugin', version: '1.0.0', description: 'Empty' }), + ); + + // Another plugin with actual skills + await createPlugin(join(testDir, 'real-plugin'), 'real-plugin', ['coding']); + + await createWorkspaceConfig(['./empty-plugin', './real-plugin']); + + // Sync + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + const skills = await getSyncedSkills(); + expect(skills).toContain('coding'); + expect(skills).toHaveLength(1); + }); + + it('should handle plugin with no skills directory', async () => { + // Setup: Plugin with commands but no skills + const pluginDir = join(testDir, 'commands-only'); + await mkdir(join(pluginDir, 'commands'), { recursive: true }); + await writeFile( + join(pluginDir, 'plugin.json'), + JSON.stringify({ name: 'commands-only', version: '1.0.0', description: 'Commands' }), + ); + await writeFile(join(pluginDir, 'commands', 'my-command.md'), '# My Command'); + + await createWorkspaceConfig(['./commands-only']); + + // Sync + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + // Verify command was copied but no skills directory created + expect(existsSync(join(testDir, '.claude', 'commands', 'my-command.md'))).toBe(true); + // Skills directory may or may not exist depending on implementation + const skills = await getSyncedSkills(); + expect(skills).toHaveLength(0); + }); + }); +}); From 3fa98c55f6c15592f9f6267df9f78ca8bb1e7c72 Mon Sep 17 00:00:00 2001 From: Christopher Date: Wed, 28 Jan 2026 12:22:05 +1100 Subject: [PATCH 6/9] docs: add duplicate skill handling documentation Explains the automatic naming resolution when multiple plugins define skills with the same folder name. Co-Authored-By: Claude Opus 4.5 --- docs/src/content/docs/guides/plugins.mdx | 30 ++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/docs/src/content/docs/guides/plugins.mdx b/docs/src/content/docs/guides/plugins.mdx index efac6929..23de568a 100644 --- a/docs/src/content/docs/guides/plugins.mdx +++ b/docs/src/content/docs/guides/plugins.mdx @@ -15,6 +15,36 @@ my-plugin/ └── AGENTS.md ``` +## Duplicate Skill Handling + +When multiple plugins define skills with the same folder name, AllAgents automatically resolves naming conflicts: + +| Conflict Type | Resolution | Example | +|---------------|------------|---------| +| Same skill folder, different plugins | `{plugin}_{skill}` | `plugin-a_coding-standards` | +| Same skill folder AND plugin name | `{org}_{plugin}_{skill}` (GitHub) | `acme_my-plugin_coding-standards` | +| Same skill folder AND plugin name | `{hash}_{plugin}_{skill}` (local) | `a1b2c3_my-plugin_coding-standards` | + +Skills with unique folder names keep their original names unchanged. + +**Example:** + +```yaml +# workspace.yaml +plugins: + - ./plugin-a # has skills/coding-standards/ + - ./plugin-b # has skills/coding-standards/ + - ./plugin-c # has skills/testing/ +``` + +After sync: +``` +.claude/skills/ +β”œβ”€β”€ plugin-a_coding-standards/ # renamed (conflict) +β”œβ”€β”€ plugin-b_coding-standards/ # renamed (conflict) +└── testing/ # unchanged (unique) +``` + ## Plugin Spec Format Plugins use the `plugin@marketplace` format: From 2535cf463477c0c3c2b6c9e247d7559ce19c5f11 Mon Sep 17 00:00:00 2001 From: Christopher Date: Wed, 28 Jan 2026 12:22:44 +1100 Subject: [PATCH 7/9] chore: remove implementation plan Plan completed and behavior documented in docs/guides/plugins.mdx Co-Authored-By: Claude Opus 4.5 --- .../2026-01-28-duplicate-skill-naming.md | 82 ------------------- 1 file changed, 82 deletions(-) delete mode 100644 .claude/plans/2026-01-28-duplicate-skill-naming.md diff --git a/.claude/plans/2026-01-28-duplicate-skill-naming.md b/.claude/plans/2026-01-28-duplicate-skill-naming.md deleted file mode 100644 index bf1f5f20..00000000 --- a/.claude/plans/2026-01-28-duplicate-skill-naming.md +++ /dev/null @@ -1,82 +0,0 @@ -# Duplicate Skill Name Handling - -## Problem - -When two plugins define skills with the same folder name, one overwrites the other during sync. There's no namespace or qualifier system to prevent collisions. - -## Naming Rules - -1. **No conflict** - Use skill folder name as-is - - Example: `coding-standards` - -2. **Skill folder name conflicts across plugins** - Qualify with plugin name using underscore separator - - Example: `foo_coding-standards` and `bar_coding-standards` - -3. **Both plugin name AND skill folder name conflict** - Add org/UUID prefix - - GitHub source: `anthropic_my-plugin_coding-standards` - - Local source: `a1b2c3_my-plugin_coding-standards` (6-char deterministic hash) - -## Detection Logic - -During sync, collect all skills from all plugins first, then: -1. Group skills by folder name -2. For groups with >1 skill, check if plugin names are unique -3. If plugin names also conflict, use org/UUID as final disambiguator - -## Implementation Approach - -### Where to change - -`src/core/transform.ts` in the `copySkills()` function (lines 156-226) - -### New logic flow - -1. **First pass** - Collect all skills from all plugins: - ``` - Map> - ``` - -2. **Build qualified names** - For each skill: - - If only one skill with that folder name β†’ use folder name as-is - - If multiple skills share folder name but different plugin names β†’ `{plugin}_{skill}` - - If multiple skills share both β†’ `{orgOrUuid}_{plugin}_{skill}` - -3. **Second pass** - Copy skills using the resolved names - -### Deriving org/UUID - -- GitHub URL source (e.g., `github:anthropic/superpowers`) β†’ extract org (`anthropic`) -- Local path source β†’ generate deterministic 6-char hash from the absolute path - -```typescript -import { createHash } from 'crypto'; - -function getShortId(localPath: string): string { - return createHash('sha256') - .update(localPath) - .digest('hex') - .substring(0, 6); -} -``` - -## Files to Modify - -| File | Change | -|------|--------| -| `src/core/transform.ts` | Refactor `copySkills()` to two-pass with name resolution | -| `src/utils/hash.ts` (new) | Small utility for 6-char deterministic hash | -| `src/utils/source-parser.ts` (new or existing) | Extract org from GitHub URLs | - -## Edge Cases - -- Plugin source URL parsing: `github:anthropic/repo` vs `github:anthropic/repo#branch` vs local paths -- Skill folder names that already contain underscores (e.g., `my_skill` from plugin `foo` β†’ `foo_my_skill`) -- Empty or missing org in GitHub URL (fallback to hash) - -## Tests to Add - -- No conflict β†’ skill name unchanged -- Skill conflict with different plugins β†’ `plugin_skill` format -- Skill + plugin conflict with GitHub sources β†’ `org_plugin_skill` format -- Skill + plugin conflict with local sources β†’ `hash_plugin_skill` format -- Mixed sources (some GitHub, some local) with conflicts From 3dbef10db0671f4754ded23ab856ba7f099523b6 Mon Sep 17 00:00:00 2001 From: Christopher Date: Wed, 28 Jan 2026 12:23:37 +1100 Subject: [PATCH 8/9] docs: clarify that plans are temporary working documents Co-Authored-By: Claude Opus 4.5 --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 426ec9f9..19cff5e4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,7 +4,7 @@ A CLI tool for managing AI coding assistant plugins across multiple clients (Cla ## Plans -Design documents and implementation plans are stored in `.claude/plans/`. +Design documents and implementation plans are stored in `.claude/plans/`. These are temporary working documents - once implementation is complete, delete the plan and update official docs with any user-facing behavior. ## Git Workflow From a932c0c698937236402aae87759ca71d642e0ff7 Mon Sep 17 00:00:00 2001 From: Christopher Date: Wed, 28 Jan 2026 12:33:12 +1100 Subject: [PATCH 9/9] refactor(tests): trim duplicate skill handling tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reduce test count from 110 to ~55: - source-parser: 38 β†’ 6 tests (consolidated assertions) - integration: 25 β†’ 7 tests (removed redundant scenarios) - Removed separate GitHub integration test file Co-Authored-By: Claude Opus 4.5 --- .../skill-duplicate-github.test.ts | 456 ------------- .../skill-duplicate-handling.test.ts | 638 +++--------------- tests/unit/utils/source-parser.test.ts | 162 +---- 3 files changed, 124 insertions(+), 1132 deletions(-) delete mode 100644 tests/integration/skill-duplicate-github.test.ts diff --git a/tests/integration/skill-duplicate-github.test.ts b/tests/integration/skill-duplicate-github.test.ts deleted file mode 100644 index 09b4e244..00000000 --- a/tests/integration/skill-duplicate-github.test.ts +++ /dev/null @@ -1,456 +0,0 @@ -/** - * Integration tests for duplicate skill handling with GitHub sources. - * - * These tests verify the org_plugin_skill naming format when plugins come from - * GitHub sources. We mock the execa module to simulate successful GitHub fetches - * pointing to local test directories. - * - * Test scenarios: - * 1. Skill + plugin conflict with GitHub sources - {org}_{plugin}_{skill} format - * 2. Mixed GitHub + local sources with conflicts - */ - -import { describe, it, expect, beforeEach, afterEach, mock } from 'bun:test'; -import { mkdtemp, rm, mkdir, writeFile, readdir, readFile } from 'node:fs/promises'; -import { existsSync } from 'node:fs'; -import { join } from 'node:path'; -import { tmpdir } from 'node:os'; -import { CONFIG_DIR, WORKSPACE_CONFIG_FILE } from '../../src/constants.js'; -import { getShortId } from '../../src/utils/hash.js'; - -// Test directory references -let testDir: string; -let cacheBaseDir: string; - -// Track which GitHub paths map to which local test paths -const githubToLocalMap = new Map(); - -// Override HOME to use test directory for cache -const originalHome = process.env.HOME; - -// Mock execa to intercept gh commands -const execaMock = mock(async (cmd: string, args: string[]) => { - if (cmd === 'gh') { - if (args[0] === '--version') { - return { stdout: 'gh version 2.40.0', stderr: '' }; - } - if (args[0] === 'repo' && args[1] === 'clone') { - // args[2] is the repo (owner/repo) - // args[3] is the destination path - const repo = args[2]; - const destPath = args[3]; - - // Find the local source for this repo - const localSource = githubToLocalMap.get(repo); - if (localSource) { - // Copy the local plugin to the cache path - await copyDir(localSource, destPath); - return { stdout: '', stderr: '' }; - } - throw new Error(`Unknown repo: ${repo}`); - } - } - throw new Error(`Unexpected command: ${cmd} ${args.join(' ')}`); -}); - -// Helper to recursively copy a directory -async function copyDir(src: string, dest: string): Promise { - await mkdir(dest, { recursive: true }); - const entries = await readdir(src, { withFileTypes: true }); - for (const entry of entries) { - const srcPath = join(src, entry.name); - const destPath = join(dest, entry.name); - if (entry.isDirectory()) { - await copyDir(srcPath, destPath); - } else { - const content = await readFile(srcPath); - await writeFile(destPath, content); - } - } -} - -// Mock the execa module BEFORE importing sync -mock.module('execa', () => ({ - execa: execaMock, -})); - -// Now import syncWorkspace after mocking -const { syncWorkspace } = await import('../../src/core/sync.js'); - -describe('Skill duplicate handling with GitHub sources', () => { - beforeEach(async () => { - testDir = await mkdtemp(join(tmpdir(), 'allagents-github-skill-')); - cacheBaseDir = join(testDir, 'home'); - - // Set HOME to our test directory so cache goes there - process.env.HOME = cacheBaseDir; - - // Clear the map and mocks - githubToLocalMap.clear(); - execaMock.mockClear(); - }); - - afterEach(async () => { - // Restore HOME - process.env.HOME = originalHome; - await rm(testDir, { recursive: true, force: true }); - }); - - /** - * Helper to create a plugin with skills - */ - async function createPlugin( - pluginPath: string, - pluginName: string, - skillNames: string[], - ): Promise { - await mkdir(pluginPath, { recursive: true }); - - // Create plugin.json - await writeFile( - join(pluginPath, 'plugin.json'), - JSON.stringify({ - name: pluginName, - version: '1.0.0', - description: `Test plugin ${pluginName}`, - }), - ); - - // Create skills - for (const skillName of skillNames) { - const skillDir = join(pluginPath, 'skills', skillName); - await mkdir(skillDir, { recursive: true }); - await writeFile( - join(skillDir, 'SKILL.md'), - `--- -name: ${skillName} -description: Skill ${skillName} from ${pluginName} ---- - -# ${skillName} - -This skill is from ${pluginName}. -`, - ); - } - } - - /** - * Helper to create workspace.yaml with specific plugin sources - */ - async function createWorkspaceConfig(plugins: string[]): Promise { - await mkdir(join(testDir, CONFIG_DIR), { recursive: true }); - await writeFile( - join(testDir, CONFIG_DIR, WORKSPACE_CONFIG_FILE), - ` -repositories: [] -plugins: -${plugins.map((p) => ` - ${p}`).join('\n')} -clients: - - claude -`, - ); - } - - /** - * Helper to get all skill directory names after sync - */ - async function getSyncedSkills(): Promise { - const skillsDir = join(testDir, '.claude', 'skills'); - if (!existsSync(skillsDir)) { - return []; - } - return readdir(skillsDir); - } - - /** - * Helper to register a GitHub source with its local cache path - * Uses gh: prefix which is recognized by both isGitHubUrl and extractOrgFromSource - */ - function registerGitHubSource(org: string, repo: string, localPath: string): string { - const key = `${org}/${repo}`; - githubToLocalMap.set(key, localPath); - return `gh:${key}`; - } - - describe('Scenario: Skill + plugin conflict with GitHub sources - {org}_{plugin}_{skill} format', () => { - it('should use org prefix when same plugin name exists in different GitHub orgs', async () => { - // Setup: Two plugins with same name from different GitHub orgs - const path1 = join(testDir, 'source', 'acme-corp', 'tools'); - const path2 = join(testDir, 'source', 'beta-inc', 'tools'); - - await createPlugin(path1, 'tools', ['deploy']); - await createPlugin(path2, 'tools', ['deploy']); - - // Register GitHub sources - const source1 = registerGitHubSource('acme-corp', 'plugins', path1); - const source2 = registerGitHubSource('beta-inc', 'plugins', path2); - - await createWorkspaceConfig([source1, source2]); - - // Sync - const result = await syncWorkspace(testDir); - - if (!result.success) { - console.error('Sync failed:', result.error); - console.error('Plugin results:', JSON.stringify(result.pluginResults, null, 2)); - } - expect(result.success).toBe(true); - - // Verify: Skills should be prefixed with org name - // Format: {org}_{plugin}_{skill} - const skills = await getSyncedSkills(); - expect(skills).toHaveLength(2); - expect(skills).toContain('acme-corp_tools_deploy'); - expect(skills).toContain('beta-inc_tools_deploy'); - }); - - it('should handle three-way plugin name conflict with GitHub orgs', async () => { - // Setup: Three plugins with same name from different orgs - const path1 = join(testDir, 'source', 'org-alpha', 'shared'); - const path2 = join(testDir, 'source', 'org-beta', 'shared'); - const path3 = join(testDir, 'source', 'org-gamma', 'shared'); - - await createPlugin(path1, 'shared-plugin', ['common-skill']); - await createPlugin(path2, 'shared-plugin', ['common-skill']); - await createPlugin(path3, 'shared-plugin', ['common-skill']); - - const source1 = registerGitHubSource('org-alpha', 'plugins', path1); - const source2 = registerGitHubSource('org-beta', 'plugins', path2); - const source3 = registerGitHubSource('org-gamma', 'plugins', path3); - - await createWorkspaceConfig([source1, source2, source3]); - - // Sync - const result = await syncWorkspace(testDir); - expect(result.success).toBe(true); - - // Verify: All three should have org prefix - const skills = await getSyncedSkills(); - expect(skills).toHaveLength(3); - expect(skills).toContain('org-alpha_shared-plugin_common-skill'); - expect(skills).toContain('org-beta_shared-plugin_common-skill'); - expect(skills).toContain('org-gamma_shared-plugin_common-skill'); - }); - - it('should preserve skill content when using org prefix', async () => { - // Setup: Two plugins with same name, different content - const path1 = join(testDir, 'source', 'company-a', 'my-plugin'); - const path2 = join(testDir, 'source', 'company-b', 'my-plugin'); - - await mkdir(join(path1, 'skills', 'coding'), { recursive: true }); - await mkdir(join(path2, 'skills', 'coding'), { recursive: true }); - - await writeFile( - join(path1, 'plugin.json'), - JSON.stringify({ name: 'my-plugin', version: '1.0.0', description: 'Company A plugin' }), - ); - await writeFile( - join(path2, 'plugin.json'), - JSON.stringify({ name: 'my-plugin', version: '1.0.0', description: 'Company B plugin' }), - ); - - await writeFile( - join(path1, 'skills', 'coding', 'SKILL.md'), - `--- -name: coding -description: Company A coding ---- -# Coding -Company A implementation. -`, - ); - - await writeFile( - join(path2, 'skills', 'coding', 'SKILL.md'), - `--- -name: coding -description: Company B coding ---- -# Coding -Company B implementation. -`, - ); - - const source1 = registerGitHubSource('company-a', 'plugins', path1); - const source2 = registerGitHubSource('company-b', 'plugins', path2); - - await createWorkspaceConfig([source1, source2]); - - // Sync - const result = await syncWorkspace(testDir); - expect(result.success).toBe(true); - - // Verify content is correct - const contentA = await readFile( - join(testDir, '.claude', 'skills', 'company-a_my-plugin_coding', 'SKILL.md'), - 'utf-8', - ); - const contentB = await readFile( - join(testDir, '.claude', 'skills', 'company-b_my-plugin_coding', 'SKILL.md'), - 'utf-8', - ); - - expect(contentA).toContain('Company A implementation'); - expect(contentB).toContain('Company B implementation'); - }); - }); - - describe('Scenario: Mixed GitHub + local sources with conflicts', () => { - it('should use org prefix for GitHub and hash prefix for local sources', async () => { - // Setup: One GitHub plugin and one local plugin with same name - const githubPath = join(testDir, 'source', 'acme', 'my-plugin'); - const localPath = join(testDir, 'local', 'my-plugin'); - - await createPlugin(githubPath, 'my-plugin', ['build']); - await createPlugin(localPath, 'my-plugin', ['build']); - - const githubSource = registerGitHubSource('acme', 'plugins', githubPath); - const localSource = './local/my-plugin'; - - await createWorkspaceConfig([githubSource, localSource]); - - // Sync - const result = await syncWorkspace(testDir); - expect(result.success).toBe(true); - - // Verify: GitHub source uses org prefix, local uses hash prefix - const skills = await getSyncedSkills(); - expect(skills).toHaveLength(2); - - // GitHub source should have org prefix - expect(skills).toContain('acme_my-plugin_build'); - - // Local source should have hash prefix - const localHash = getShortId(localSource); - expect(skills).toContain(`${localHash}_my-plugin_build`); - }); - - it('should handle mixed sources where only some conflict', async () => { - // Setup: - // - GitHub plugin from acme: has "shared" and "unique-github" skills - // - Local plugin: has "shared" and "unique-local" skills - // - Another GitHub plugin from beta: has "other" skill (no conflict) - - const acmePath = join(testDir, 'source', 'acme', 'devtools'); - const localPath = join(testDir, 'local', 'devtools'); - const betaPath = join(testDir, 'source', 'beta', 'utilities'); - - await createPlugin(acmePath, 'devtools', ['shared', 'unique-github']); - await createPlugin(localPath, 'devtools', ['shared', 'unique-local']); - await createPlugin(betaPath, 'utilities', ['other']); - - const acmeSource = registerGitHubSource('acme', 'devtools', acmePath); - const betaSource = registerGitHubSource('beta', 'utilities', betaPath); - const localSource = './local/devtools'; - - await createWorkspaceConfig([acmeSource, localSource, betaSource]); - - // Sync - const result = await syncWorkspace(testDir); - expect(result.success).toBe(true); - - const skills = await getSyncedSkills(); - - // Unique skills should keep original names - expect(skills).toContain('unique-github'); - expect(skills).toContain('unique-local'); - expect(skills).toContain('other'); - - // Conflicting skills should be fully qualified - // Both have same plugin name "devtools" and same skill "shared" - const localHash = getShortId(localSource); - expect(skills).toContain('acme_devtools_shared'); - expect(skills).toContain(`${localHash}_devtools_shared`); - }); - - it('should handle multiple GitHub orgs and multiple local paths all conflicting', async () => { - // Setup: Four plugins all with same name and same skill - const acmePath = join(testDir, 'source', 'acme', 'builder'); - const betaPath = join(testDir, 'source', 'beta', 'builder'); - const local1Path = join(testDir, 'vendor', 'builder'); - const local2Path = join(testDir, 'custom', 'builder'); - - await createPlugin(acmePath, 'builder', ['compile']); - await createPlugin(betaPath, 'builder', ['compile']); - await createPlugin(local1Path, 'builder', ['compile']); - await createPlugin(local2Path, 'builder', ['compile']); - - const acmeSource = registerGitHubSource('acme', 'builder', acmePath); - const betaSource = registerGitHubSource('beta', 'builder', betaPath); - const local1Source = './vendor/builder'; - const local2Source = './custom/builder'; - - await createWorkspaceConfig([acmeSource, betaSource, local1Source, local2Source]); - - // Sync - const result = await syncWorkspace(testDir); - expect(result.success).toBe(true); - - const skills = await getSyncedSkills(); - expect(skills).toHaveLength(4); - - // GitHub sources use org prefix - expect(skills).toContain('acme_builder_compile'); - expect(skills).toContain('beta_builder_compile'); - - // Local sources use hash prefix - const hash1 = getShortId(local1Source); - const hash2 = getShortId(local2Source); - expect(skills).toContain(`${hash1}_builder_compile`); - expect(skills).toContain(`${hash2}_builder_compile`); - }); - - it('should track renamed skills correctly in sync state for mixed sources', async () => { - // Setup: One GitHub and one local with same plugin name - const githubPath = join(testDir, 'source', 'myorg', 'analyzer'); - const localPath = join(testDir, 'local', 'analyzer'); - - await createPlugin(githubPath, 'analyzer', ['lint']); - await createPlugin(localPath, 'analyzer', ['lint']); - - const githubSource = registerGitHubSource('myorg', 'analyzer', githubPath); - const localSource = './local/analyzer'; - - await createWorkspaceConfig([githubSource, localSource]); - - // Sync - await syncWorkspace(testDir); - - // Verify state file contains renamed skill paths - const statePath = join(testDir, CONFIG_DIR, 'sync-state.json'); - const stateContent = await readFile(statePath, 'utf-8'); - const state = JSON.parse(stateContent); - - const localHash = getShortId(localSource); - - // Both renamed paths should be in state - expect(state.files.claude).toContain('.claude/skills/myorg_analyzer_lint/'); - expect(state.files.claude).toContain(`.claude/skills/${localHash}_analyzer_lint/`); - }); - }); - - describe('Scenario: GitHub shorthand formats', () => { - it('should extract org from gh: shorthand format', async () => { - // Setup: Using gh: prefix (registered via helper) - const path1 = join(testDir, 'source', 'team-x', 'tools'); - const path2 = join(testDir, 'source', 'team-y', 'tools'); - - await createPlugin(path1, 'tools', ['run']); - await createPlugin(path2, 'tools', ['run']); - - const source1 = registerGitHubSource('team-x', 'tools', path1); - const source2 = registerGitHubSource('team-y', 'tools', path2); - - await createWorkspaceConfig([source1, source2]); - - // Sync - const result = await syncWorkspace(testDir); - expect(result.success).toBe(true); - - const skills = await getSyncedSkills(); - expect(skills).toContain('team-x_tools_run'); - expect(skills).toContain('team-y_tools_run'); - }); - }); -}); diff --git a/tests/integration/skill-duplicate-handling.test.ts b/tests/integration/skill-duplicate-handling.test.ts index 00f82a08..eae1abc0 100644 --- a/tests/integration/skill-duplicate-handling.test.ts +++ b/tests/integration/skill-duplicate-handling.test.ts @@ -1,17 +1,9 @@ /** * Integration tests for duplicate skill handling in the full sync flow. - * - * These tests verify that when multiple plugins have skills with the same - * folder name, the sync correctly disambiguates them based on: - * 1. No conflict - skill name unchanged - * 2. Skill conflict with different plugins - {plugin}_{skill} format - * 3. Skill + plugin conflict with GitHub sources - {org}_{plugin}_{skill} format - * 4. Skill + plugin conflict with local sources - {hash}_{plugin}_{skill} format - * 5. Mixed sources (some GitHub, some local) with conflicts */ import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; -import { mkdtemp, rm, mkdir, writeFile, readdir, readFile } from 'node:fs/promises'; +import { mkdtemp, rm, mkdir, writeFile, readdir } from 'node:fs/promises'; import { existsSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; @@ -19,7 +11,7 @@ import { syncWorkspace } from '../../src/core/sync.js'; import { CONFIG_DIR, WORKSPACE_CONFIG_FILE } from '../../src/constants.js'; import { getShortId } from '../../src/utils/hash.js'; -describe('Skill duplicate handling integration tests', () => { +describe('Skill duplicate handling', () => { let testDir: string; beforeEach(async () => { @@ -30,560 +22,146 @@ describe('Skill duplicate handling integration tests', () => { await rm(testDir, { recursive: true, force: true }); }); - /** - * Helper to create a plugin with skills - */ - async function createPlugin( - pluginPath: string, - pluginName: string, - skillNames: string[], - ): Promise { - await mkdir(pluginPath, { recursive: true }); - - // Create plugin.json - await writeFile( - join(pluginPath, 'plugin.json'), - JSON.stringify({ - name: pluginName, - version: '1.0.0', - description: `Test plugin ${pluginName}`, - }), - ); - - // Create skills - for (const skillName of skillNames) { - const skillDir = join(pluginPath, 'skills', skillName); + async function createPlugin(path: string, name: string, skills: string[]): Promise { + await mkdir(path, { recursive: true }); + await writeFile(join(path, 'plugin.json'), JSON.stringify({ name, version: '1.0.0' })); + for (const skill of skills) { + const skillDir = join(path, 'skills', skill); await mkdir(skillDir, { recursive: true }); - await writeFile( - join(skillDir, 'SKILL.md'), - `--- -name: ${skillName} -description: Skill ${skillName} from ${pluginName} ---- - -# ${skillName} - -This skill is from ${pluginName}. -`, - ); + await writeFile(join(skillDir, 'SKILL.md'), `---\nname: ${skill}\ndescription: Test skill\n---\n# ${skill}`); } } - /** - * Helper to create workspace.yaml - */ async function createWorkspaceConfig(plugins: string[]): Promise { await mkdir(join(testDir, CONFIG_DIR), { recursive: true }); await writeFile( join(testDir, CONFIG_DIR, WORKSPACE_CONFIG_FILE), - ` -repositories: [] -plugins: -${plugins.map((p) => ` - ${p}`).join('\n')} -clients: - - claude -`, + `repositories: []\nplugins:\n${plugins.map((p) => ` - ${p}`).join('\n')}\nclients:\n - claude`, ); } - /** - * Helper to get all skill directory names after sync - */ async function getSyncedSkills(): Promise { - const skillsDir = join(testDir, '.claude', 'skills'); - if (!existsSync(skillsDir)) { - return []; - } - return readdir(skillsDir); + const dir = join(testDir, '.claude', 'skills'); + return existsSync(dir) ? readdir(dir) : []; } - describe('Scenario 1: No conflict - skill name unchanged', () => { - it('should keep original folder names when no skills conflict', async () => { - // Setup: Two plugins with unique skill names - await createPlugin(join(testDir, 'plugin-alpha'), 'plugin-alpha', [ - 'skill-a', - 'skill-b', - ]); - await createPlugin(join(testDir, 'plugin-beta'), 'plugin-beta', [ - 'skill-c', - 'skill-d', - ]); - - await createWorkspaceConfig(['./plugin-alpha', './plugin-beta']); - - // Sync - const result = await syncWorkspace(testDir); - expect(result.success).toBe(true); - - // Verify skills keep original names - const skills = await getSyncedSkills(); - expect(skills.sort()).toEqual(['skill-a', 'skill-b', 'skill-c', 'skill-d']); - }); - - it('should handle single plugin with multiple unique skills', async () => { - // Setup: One plugin with multiple skills - await createPlugin(join(testDir, 'my-plugin'), 'my-plugin', [ - 'coding', - 'testing', - 'debugging', - ]); - - await createWorkspaceConfig(['./my-plugin']); + it('should keep original names when no conflicts', async () => { + await createPlugin(join(testDir, 'plugin-a'), 'plugin-a', ['skill-a']); + await createPlugin(join(testDir, 'plugin-b'), 'plugin-b', ['skill-b']); + await createWorkspaceConfig(['./plugin-a', './plugin-b']); - // Sync - const result = await syncWorkspace(testDir); - expect(result.success).toBe(true); + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); - // Verify skills keep original names - const skills = await getSyncedSkills(); - expect(skills.sort()).toEqual(['coding', 'debugging', 'testing']); - }); + const skills = await getSyncedSkills(); + expect(skills.sort()).toEqual(['skill-a', 'skill-b']); }); - describe('Scenario 2: Skill conflict with different plugin names - {plugin}_{skill} format', () => { - it('should qualify skill names with plugin prefix when folder names conflict', async () => { - // Setup: Two plugins with same skill folder name but different plugin names - await createPlugin(join(testDir, 'alpha-tools'), 'alpha-tools', ['common-skill']); - await createPlugin(join(testDir, 'beta-tools'), 'beta-tools', ['common-skill']); + it('should qualify with plugin name when skill folders conflict', async () => { + await createPlugin(join(testDir, 'alpha'), 'alpha', ['common']); + await createPlugin(join(testDir, 'beta'), 'beta', ['common']); + await createWorkspaceConfig(['./alpha', './beta']); - await createWorkspaceConfig(['./alpha-tools', './beta-tools']); + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); - // Sync - const result = await syncWorkspace(testDir); - expect(result.success).toBe(true); - - // Verify skills are qualified with plugin name - const skills = await getSyncedSkills(); - expect(skills.sort()).toEqual(['alpha-tools_common-skill', 'beta-tools_common-skill']); - }); - - it('should only rename conflicting skills, not unique ones', async () => { - // Setup: Two plugins, one skill conflicts, others unique - await createPlugin(join(testDir, 'plugin-one'), 'plugin-one', [ - 'unique-to-one', - 'shared-skill', - ]); - await createPlugin(join(testDir, 'plugin-two'), 'plugin-two', [ - 'shared-skill', - 'unique-to-two', - ]); - - await createWorkspaceConfig(['./plugin-one', './plugin-two']); - - // Sync - const result = await syncWorkspace(testDir); - expect(result.success).toBe(true); - - // Verify: unique skills unchanged, shared skills qualified - const skills = await getSyncedSkills(); - expect(skills).toContain('unique-to-one'); - expect(skills).toContain('unique-to-two'); - expect(skills).toContain('plugin-one_shared-skill'); - expect(skills).toContain('plugin-two_shared-skill'); - expect(skills).not.toContain('shared-skill'); // Should be qualified - }); - - it('should handle multiple conflicting skills across three plugins', async () => { - // Setup: Three plugins with overlapping skills - await createPlugin(join(testDir, 'plugin-a'), 'plugin-a', ['common', 'special-a']); - await createPlugin(join(testDir, 'plugin-b'), 'plugin-b', ['common', 'special-b']); - await createPlugin(join(testDir, 'plugin-c'), 'plugin-c', ['common', 'special-c']); - - await createWorkspaceConfig(['./plugin-a', './plugin-b', './plugin-c']); - - // Sync - const result = await syncWorkspace(testDir); - expect(result.success).toBe(true); - - // Verify: unique skills unchanged, common skill qualified for all three - const skills = await getSyncedSkills(); - expect(skills).toContain('special-a'); - expect(skills).toContain('special-b'); - expect(skills).toContain('special-c'); - expect(skills).toContain('plugin-a_common'); - expect(skills).toContain('plugin-b_common'); - expect(skills).toContain('plugin-c_common'); - }); + const skills = await getSyncedSkills(); + expect(skills.sort()).toEqual(['alpha_common', 'beta_common']); }); - describe('Scenario 3: Skill + plugin conflict with local sources - {hash}_{plugin}_{skill} format', () => { - it('should add hash prefix when both skill folder and plugin name conflict (local paths)', async () => { - // Setup: Two plugins from DIFFERENT local paths with SAME plugin name - // This simulates having two different versions/forks of the same plugin - const path1 = join(testDir, 'vendor', 'acme', 'my-plugin'); - const path2 = join(testDir, 'local', 'custom', 'my-plugin'); - - await createPlugin(path1, 'my-plugin', ['coding']); - await createPlugin(path2, 'my-plugin', ['coding']); - - // Workspace config uses relative paths from testDir - await createWorkspaceConfig(['./vendor/acme/my-plugin', './local/custom/my-plugin']); - - // Sync - const result = await syncWorkspace(testDir); - expect(result.success).toBe(true); + it('should only rename conflicting skills', async () => { + await createPlugin(join(testDir, 'plugin-a'), 'plugin-a', ['unique-a', 'shared']); + await createPlugin(join(testDir, 'plugin-b'), 'plugin-b', ['shared', 'unique-b']); + await createWorkspaceConfig(['./plugin-a', './plugin-b']); - // Verify: Skills should be prefixed with hash of the source path - const skills = await getSyncedSkills(); - expect(skills).toHaveLength(2); + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); - // Both skills should have the pattern: {6-char-hash}_my-plugin_coding - // The hash is based on the plugin source string - const hash1 = getShortId('./vendor/acme/my-plugin'); - const hash2 = getShortId('./local/custom/my-plugin'); - - expect(skills).toContain(`${hash1}_my-plugin_coding`); - expect(skills).toContain(`${hash2}_my-plugin_coding`); - }); - - it('should handle multiple skills with full disambiguation', async () => { - // Setup: Two plugins from different paths, same name, multiple conflicting skills - const path1 = join(testDir, 'team-a', 'shared-plugin'); - const path2 = join(testDir, 'team-b', 'shared-plugin'); - - await createPlugin(path1, 'shared-plugin', ['analyze', 'generate', 'validate']); - await createPlugin(path2, 'shared-plugin', ['analyze', 'transform', 'validate']); - - await createWorkspaceConfig(['./team-a/shared-plugin', './team-b/shared-plugin']); - - // Sync - const result = await syncWorkspace(testDir); - expect(result.success).toBe(true); - - const skills = await getSyncedSkills(); - - // Unique skills should keep original names - expect(skills).toContain('generate'); - expect(skills).toContain('transform'); - - // Conflicting skills should have hash prefix - const hash1 = getShortId('./team-a/shared-plugin'); - const hash2 = getShortId('./team-b/shared-plugin'); - - expect(skills).toContain(`${hash1}_shared-plugin_analyze`); - expect(skills).toContain(`${hash2}_shared-plugin_analyze`); - expect(skills).toContain(`${hash1}_shared-plugin_validate`); - expect(skills).toContain(`${hash2}_shared-plugin_validate`); - }); + const skills = await getSyncedSkills(); + expect(skills).toContain('unique-a'); + expect(skills).toContain('unique-b'); + expect(skills).toContain('plugin-a_shared'); + expect(skills).toContain('plugin-b_shared'); + expect(skills).not.toContain('shared'); }); - describe('Scenario 4: Mixed conflict levels', () => { - it('should handle mix of no-conflict, plugin-conflict, and full-conflict skills', async () => { - // Setup: - // - plugin-unique: has "unique-skill" (no conflict) - // - plugin-alpha: has "shared-skill" (conflicts with beta, different plugin names) - // - plugin-beta: has "shared-skill" (conflicts with alpha) - // - same-name plugins from different paths with "common" skill (full conflict) - - await createPlugin(join(testDir, 'plugin-unique'), 'plugin-unique', ['unique-skill']); - await createPlugin(join(testDir, 'plugin-alpha'), 'plugin-alpha', ['shared-skill']); - await createPlugin(join(testDir, 'plugin-beta'), 'plugin-beta', ['shared-skill']); - await createPlugin(join(testDir, 'path-a', 'forked-plugin'), 'forked-plugin', ['common']); - await createPlugin(join(testDir, 'path-b', 'forked-plugin'), 'forked-plugin', ['common']); - - await createWorkspaceConfig([ - './plugin-unique', - './plugin-alpha', - './plugin-beta', - './path-a/forked-plugin', - './path-b/forked-plugin', - ]); - - // Sync - const result = await syncWorkspace(testDir); - expect(result.success).toBe(true); - - const skills = await getSyncedSkills(); - - // No conflict - keeps original name - expect(skills).toContain('unique-skill'); - - // Plugin-level conflict - {plugin}_{skill} - expect(skills).toContain('plugin-alpha_shared-skill'); - expect(skills).toContain('plugin-beta_shared-skill'); - - // Full conflict - {hash}_{plugin}_{skill} - const hashA = getShortId('./path-a/forked-plugin'); - const hashB = getShortId('./path-b/forked-plugin'); - expect(skills).toContain(`${hashA}_forked-plugin_common`); - expect(skills).toContain(`${hashB}_forked-plugin_common`); - }); + it('should add hash prefix when both skill and plugin names conflict', async () => { + const path1 = join(testDir, 'vendor', 'my-plugin'); + const path2 = join(testDir, 'local', 'my-plugin'); + await createPlugin(path1, 'my-plugin', ['build']); + await createPlugin(path2, 'my-plugin', ['build']); + await createWorkspaceConfig(['./vendor/my-plugin', './local/my-plugin']); + + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + const skills = await getSyncedSkills(); + const hash1 = getShortId('./vendor/my-plugin'); + const hash2 = getShortId('./local/my-plugin'); + expect(skills).toContain(`${hash1}_my-plugin_build`); + expect(skills).toContain(`${hash2}_my-plugin_build`); }); - describe('Scenario 5: State tracking and purging of renamed skills', () => { - it('should correctly track renamed skills in sync state', async () => { - // Setup: Two plugins with conflicting skills - await createPlugin(join(testDir, 'plugin-a'), 'plugin-a', ['shared']); - await createPlugin(join(testDir, 'plugin-b'), 'plugin-b', ['shared']); - - await createWorkspaceConfig(['./plugin-a', './plugin-b']); - - // First sync - await syncWorkspace(testDir); - - // Verify state file contains renamed skill paths - const statePath = join(testDir, CONFIG_DIR, 'sync-state.json'); - const stateContent = await readFile(statePath, 'utf-8'); - const state = JSON.parse(stateContent); - - expect(state.files.claude).toContain('.claude/skills/plugin-a_shared/'); - expect(state.files.claude).toContain('.claude/skills/plugin-b_shared/'); - }); - - it('should purge renamed skills and revert to original names when conflict resolves', async () => { - // Setup: Two plugins with conflicting skills - await createPlugin(join(testDir, 'plugin-a'), 'plugin-a', ['shared']); - await createPlugin(join(testDir, 'plugin-b'), 'plugin-b', ['shared']); - - await createWorkspaceConfig(['./plugin-a', './plugin-b']); - - // First sync - should create qualified names - await syncWorkspace(testDir); - - let skills = await getSyncedSkills(); - expect(skills).toContain('plugin-a_shared'); - expect(skills).toContain('plugin-b_shared'); - - // Remove plugin-b from workspace - await createWorkspaceConfig(['./plugin-a']); - - // Second sync - conflict resolved, should use original name - await syncWorkspace(testDir); - - skills = await getSyncedSkills(); - expect(skills).toContain('shared'); // Reverted to original - expect(skills).not.toContain('plugin-a_shared'); // Old qualified name gone - expect(skills).not.toContain('plugin-b_shared'); // Removed plugin's skill gone - }); - - it('should handle transition from non-conflicting to conflicting state', async () => { - // Setup: Start with one plugin - await createPlugin(join(testDir, 'plugin-a'), 'plugin-a', ['coding']); - - await createWorkspaceConfig(['./plugin-a']); - - // First sync - no conflict - await syncWorkspace(testDir); - - let skills = await getSyncedSkills(); - expect(skills).toContain('coding'); - - // Add second plugin with same skill name - await createPlugin(join(testDir, 'plugin-b'), 'plugin-b', ['coding']); - await createWorkspaceConfig(['./plugin-a', './plugin-b']); - - // Second sync - now has conflict - await syncWorkspace(testDir); - - skills = await getSyncedSkills(); - expect(skills).not.toContain('coding'); // Original name should be gone - expect(skills).toContain('plugin-a_coding'); - expect(skills).toContain('plugin-b_coding'); - }); + it('should handle mixed conflict levels', async () => { + await createPlugin(join(testDir, 'unique-plugin'), 'unique-plugin', ['unique']); + await createPlugin(join(testDir, 'alpha'), 'alpha', ['shared']); + await createPlugin(join(testDir, 'beta'), 'beta', ['shared']); + await createPlugin(join(testDir, 'path-a', 'fork'), 'fork', ['common']); + await createPlugin(join(testDir, 'path-b', 'fork'), 'fork', ['common']); + await createWorkspaceConfig([ + './unique-plugin', + './alpha', + './beta', + './path-a/fork', + './path-b/fork', + ]); + + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + const skills = await getSyncedSkills(); + expect(skills).toContain('unique'); // no conflict + expect(skills).toContain('alpha_shared'); // plugin conflict + expect(skills).toContain('beta_shared'); + const hashA = getShortId('./path-a/fork'); + const hashB = getShortId('./path-b/fork'); + expect(skills).toContain(`${hashA}_fork_common`); // full conflict + expect(skills).toContain(`${hashB}_fork_common`); }); - describe('Scenario 6: Skill content verification', () => { - it('should preserve skill content when renaming', async () => { - // Setup: Two plugins with conflicting skill names - await createPlugin(join(testDir, 'plugin-alpha'), 'plugin-alpha', ['common']); - await createPlugin(join(testDir, 'plugin-beta'), 'plugin-beta', ['common']); - - // Modify skill content to be unique - await writeFile( - join(testDir, 'plugin-alpha', 'skills', 'common', 'SKILL.md'), - `--- -name: common -description: Alpha version ---- - -# Common Skill - -This is the ALPHA version. -`, - ); - - await writeFile( - join(testDir, 'plugin-beta', 'skills', 'common', 'SKILL.md'), - `--- -name: common -description: Beta version ---- - -# Common Skill - -This is the BETA version. -`, - ); - - await createWorkspaceConfig(['./plugin-alpha', './plugin-beta']); - - // Sync - const result = await syncWorkspace(testDir); - expect(result.success).toBe(true); - - // Verify content is preserved correctly - const alphaContent = await readFile( - join(testDir, '.claude', 'skills', 'plugin-alpha_common', 'SKILL.md'), - 'utf-8', - ); - const betaContent = await readFile( - join(testDir, '.claude', 'skills', 'plugin-beta_common', 'SKILL.md'), - 'utf-8', - ); - - expect(alphaContent).toContain('ALPHA version'); - expect(betaContent).toContain('BETA version'); - }); - - it('should copy all files in skill directory when renaming', async () => { - // Setup: Plugin with skill containing multiple files - const pluginDir = join(testDir, 'plugin-alpha'); - const skillDir = join(pluginDir, 'skills', 'complex-skill'); - - await mkdir(skillDir, { recursive: true }); - await writeFile( - join(pluginDir, 'plugin.json'), - JSON.stringify({ name: 'plugin-alpha', version: '1.0.0', description: 'Alpha' }), - ); - await writeFile( - join(skillDir, 'SKILL.md'), - `--- -name: complex-skill -description: A complex skill ---- - -# Complex Skill -`, - ); - await writeFile(join(skillDir, 'helper.py'), '# Helper script'); - await writeFile(join(skillDir, 'config.json'), '{"setting": true}'); - - // Second plugin with same skill name - await createPlugin(join(testDir, 'plugin-beta'), 'plugin-beta', ['complex-skill']); + it('should revert to original name when conflict resolves', async () => { + await createPlugin(join(testDir, 'plugin-a'), 'plugin-a', ['coding']); + await createPlugin(join(testDir, 'plugin-b'), 'plugin-b', ['coding']); + await createWorkspaceConfig(['./plugin-a', './plugin-b']); - await createWorkspaceConfig(['./plugin-alpha', './plugin-beta']); + await syncWorkspace(testDir); + let skills = await getSyncedSkills(); + expect(skills).toContain('plugin-a_coding'); + expect(skills).toContain('plugin-b_coding'); - // Sync - const result = await syncWorkspace(testDir); - expect(result.success).toBe(true); + // Remove conflict + await createWorkspaceConfig(['./plugin-a']); + await syncWorkspace(testDir); - // Verify all files are copied to renamed directory - const renamedDir = join(testDir, '.claude', 'skills', 'plugin-alpha_complex-skill'); - expect(existsSync(join(renamedDir, 'SKILL.md'))).toBe(true); - expect(existsSync(join(renamedDir, 'helper.py'))).toBe(true); - expect(existsSync(join(renamedDir, 'config.json'))).toBe(true); - }); + skills = await getSyncedSkills(); + expect(skills).toContain('coding'); + expect(skills).not.toContain('plugin-a_coding'); }); - describe('Scenario 7: Edge cases', () => { - it('should handle plugins without plugin.json (fallback to directory name)', async () => { - // Setup: Plugins without plugin.json - const plugin1Dir = join(testDir, 'tools-alpha'); - const plugin2Dir = join(testDir, 'tools-beta'); - - await mkdir(join(plugin1Dir, 'skills', 'shared'), { recursive: true }); - await mkdir(join(plugin2Dir, 'skills', 'shared'), { recursive: true }); - - // No plugin.json - should use directory name - await writeFile( - join(plugin1Dir, 'skills', 'shared', 'SKILL.md'), - `--- -name: shared -description: Shared from alpha ----`, - ); - await writeFile( - join(plugin2Dir, 'skills', 'shared', 'SKILL.md'), - `--- -name: shared -description: Shared from beta ----`, - ); - - await createWorkspaceConfig(['./tools-alpha', './tools-beta']); - - // Sync - const result = await syncWorkspace(testDir); - expect(result.success).toBe(true); - - // Verify skills are qualified with directory name - const skills = await getSyncedSkills(); - expect(skills).toContain('tools-alpha_shared'); - expect(skills).toContain('tools-beta_shared'); - }); - - it('should handle skill names with hyphens correctly', async () => { - // Setup: Skills with various hyphenated naming conventions - // Note: Underscores are NOT allowed in skill names per validation rules - await createPlugin(join(testDir, 'plugin-a'), 'plugin-a', [ - 'my-skill', - 'my-other-skill', - 'my-unique-skill', - ]); - await createPlugin(join(testDir, 'plugin-b'), 'plugin-b', ['my-skill', 'my-other-skill']); - - await createWorkspaceConfig(['./plugin-a', './plugin-b']); - - // Sync - const result = await syncWorkspace(testDir); - expect(result.success).toBe(true); - - const skills = await getSyncedSkills(); - - // Unique skill keeps original name - expect(skills).toContain('my-unique-skill'); - - // Conflicting skills are qualified - expect(skills).toContain('plugin-a_my-skill'); - expect(skills).toContain('plugin-b_my-skill'); - expect(skills).toContain('plugin-a_my-other-skill'); - expect(skills).toContain('plugin-b_my-other-skill'); - }); - - it('should handle empty skills directory gracefully', async () => { - // Setup: Plugin with skills directory but no skills - const pluginDir = join(testDir, 'empty-plugin'); - await mkdir(join(pluginDir, 'skills'), { recursive: true }); - await writeFile( - join(pluginDir, 'plugin.json'), - JSON.stringify({ name: 'empty-plugin', version: '1.0.0', description: 'Empty' }), - ); - - // Another plugin with actual skills - await createPlugin(join(testDir, 'real-plugin'), 'real-plugin', ['coding']); - - await createWorkspaceConfig(['./empty-plugin', './real-plugin']); - - // Sync - const result = await syncWorkspace(testDir); - expect(result.success).toBe(true); - - const skills = await getSyncedSkills(); - expect(skills).toContain('coding'); - expect(skills).toHaveLength(1); - }); - - it('should handle plugin with no skills directory', async () => { - // Setup: Plugin with commands but no skills - const pluginDir = join(testDir, 'commands-only'); - await mkdir(join(pluginDir, 'commands'), { recursive: true }); - await writeFile( - join(pluginDir, 'plugin.json'), - JSON.stringify({ name: 'commands-only', version: '1.0.0', description: 'Commands' }), - ); - await writeFile(join(pluginDir, 'commands', 'my-command.md'), '# My Command'); - - await createWorkspaceConfig(['./commands-only']); - - // Sync - const result = await syncWorkspace(testDir); - expect(result.success).toBe(true); - - // Verify command was copied but no skills directory created - expect(existsSync(join(testDir, '.claude', 'commands', 'my-command.md'))).toBe(true); - // Skills directory may or may not exist depending on implementation - const skills = await getSyncedSkills(); - expect(skills).toHaveLength(0); - }); + it('should use directory name when plugin.json is missing', async () => { + const dir1 = join(testDir, 'tools-alpha'); + const dir2 = join(testDir, 'tools-beta'); + await mkdir(join(dir1, 'skills', 'shared'), { recursive: true }); + await mkdir(join(dir2, 'skills', 'shared'), { recursive: true }); + await writeFile(join(dir1, 'skills', 'shared', 'SKILL.md'), '---\nname: shared\ndescription: Test\n---'); + await writeFile(join(dir2, 'skills', 'shared', 'SKILL.md'), '---\nname: shared\ndescription: Test\n---'); + await createWorkspaceConfig(['./tools-alpha', './tools-beta']); + + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + const skills = await getSyncedSkills(); + expect(skills).toContain('tools-alpha_shared'); + expect(skills).toContain('tools-beta_shared'); }); }); diff --git a/tests/unit/utils/source-parser.test.ts b/tests/unit/utils/source-parser.test.ts index 698dcf14..928b4dd6 100644 --- a/tests/unit/utils/source-parser.test.ts +++ b/tests/unit/utils/source-parser.test.ts @@ -2,179 +2,49 @@ import { describe, it, expect } from 'bun:test'; import { extractOrgFromSource } from '../../../src/utils/source-parser.js'; describe('extractOrgFromSource', () => { - describe('github: prefix format', () => { - it('should extract org from github:org/repo', () => { + describe('GitHub formats', () => { + it('should extract org from github: and gh: prefix formats', () => { expect(extractOrgFromSource('github:anthropic/repo')).toBe('anthropic'); - }); - - it('should extract org from github:org/repo#branch', () => { expect(extractOrgFromSource('github:anthropic/repo#main')).toBe('anthropic'); - expect(extractOrgFromSource('github:anthropic/repo#feature/my-branch')).toBe('anthropic'); + expect(extractOrgFromSource('gh:my-org/repo')).toBe('my-org'); + expect(extractOrgFromSource('gh:my-org/repo#branch')).toBe('my-org'); }); - it('should handle hyphenated org names', () => { - expect(extractOrgFromSource('github:my-org/repo')).toBe('my-org'); - expect(extractOrgFromSource('github:my-org/repo#branch')).toBe('my-org'); - }); - - it('should handle numeric org names', () => { - expect(extractOrgFromSource('github:org123/repo')).toBe('org123'); - }); - - it('should handle single character org names', () => { - expect(extractOrgFromSource('github:a/repo')).toBe('a'); - }); - }); - - describe('gh: prefix format', () => { - it('should extract org from gh:org/repo', () => { - expect(extractOrgFromSource('gh:anthropic/repo')).toBe('anthropic'); - }); - - it('should extract org from gh:org/repo#branch', () => { - expect(extractOrgFromSource('gh:anthropic/repo#develop')).toBe('anthropic'); - }); - }); - - describe('full GitHub URL format', () => { - it('should extract org from https://github.com/org/repo', () => { + it('should extract org from full GitHub URLs', () => { expect(extractOrgFromSource('https://github.com/anthropic/claude-code')).toBe('anthropic'); + expect(extractOrgFromSource('github.com/EntityProcess/allagents')).toBe('EntityProcess'); + expect(extractOrgFromSource('https://github.com/org/repo/tree/main/path')).toBe('org'); }); - it('should extract org from http://github.com/org/repo', () => { - expect(extractOrgFromSource('http://github.com/anthropic/repo')).toBe('anthropic'); - }); - - it('should extract org from https://www.github.com/org/repo', () => { - expect(extractOrgFromSource('https://www.github.com/EntityProcess/allagents')).toBe('EntityProcess'); - }); - - it('should extract org from github.com/org/repo (no protocol)', () => { - expect(extractOrgFromSource('github.com/anthropic/repo')).toBe('anthropic'); - }); - - it('should extract org from URLs with tree/branch/path', () => { - expect(extractOrgFromSource('https://github.com/anthropic/repo/tree/main/plugins')).toBe('anthropic'); - }); - }); - - describe('shorthand format (org/repo)', () => { - it('should extract org from owner/repo', () => { - expect(extractOrgFromSource('anthropic/claude-plugins-official')).toBe('anthropic'); - }); - - it('should extract org from owner/repo/subpath', () => { - expect(extractOrgFromSource('anthropic/repo/plugins/code-review')).toBe('anthropic'); - }); - - it('should handle dots in repo names', () => { + it('should extract org from shorthand format (org/repo)', () => { + expect(extractOrgFromSource('anthropic/claude-plugins')).toBe('anthropic'); expect(extractOrgFromSource('WiseTechGlobal/WTG.AI.Prompts')).toBe('WiseTechGlobal'); }); }); describe('local paths - should return null', () => { - it('should return null for relative paths starting with ./', () => { + it('should return null for local paths', () => { expect(extractOrgFromSource('./local/path')).toBeNull(); - }); - - it('should return null for relative paths starting with ../', () => { expect(extractOrgFromSource('../parent/path')).toBeNull(); - }); - - it('should return null for absolute Unix paths', () => { - expect(extractOrgFromSource('/home/user/plugins/skill')).toBeNull(); - }); - - it('should return null for Windows absolute paths', () => { + expect(extractOrgFromSource('/home/user/plugins')).toBeNull(); expect(extractOrgFromSource('C:\\Users\\test\\plugins')).toBeNull(); - expect(extractOrgFromSource('D:/Projects/plugins')).toBeNull(); - }); - - it('should return null for paths with backslashes', () => { - expect(extractOrgFromSource('local\\path\\to\\plugin')).toBeNull(); - }); - - it('should return null for current directory', () => { expect(extractOrgFromSource('.')).toBeNull(); }); - - it('should return null for parent directory', () => { - expect(extractOrgFromSource('..')).toBeNull(); - }); }); - describe('invalid or empty input - should return null', () => { - it('should return null for empty string', () => { + describe('invalid input - should return null', () => { + it('should return null for invalid input', () => { expect(extractOrgFromSource('')).toBeNull(); - }); - - it('should return null for whitespace-only string', () => { expect(extractOrgFromSource(' ')).toBeNull(); - expect(extractOrgFromSource('\t')).toBeNull(); - expect(extractOrgFromSource('\n')).toBeNull(); - }); - - it('should return null for github: with no repo', () => { expect(extractOrgFromSource('github:')).toBeNull(); - }); - - it('should return null for github: with empty org', () => { expect(extractOrgFromSource('github:/repo')).toBeNull(); - }); - - it('should return null for github: with only org (no slash)', () => { - expect(extractOrgFromSource('github:anthropic')).toBeNull(); - }); - - it('should return null for invalid org starting with hyphen', () => { expect(extractOrgFromSource('github:-invalid/repo')).toBeNull(); - }); - - it('should return null for invalid org ending with hyphen', () => { - expect(extractOrgFromSource('github:invalid-/repo')).toBeNull(); - }); - - it('should return null for org with consecutive hyphens', () => { expect(extractOrgFromSource('github:in--valid/repo')).toBeNull(); }); - - it('should return null for org with special characters', () => { - expect(extractOrgFromSource('github:org@name/repo')).toBeNull(); - expect(extractOrgFromSource('github:org.name/repo')).toBeNull(); - expect(extractOrgFromSource('github:org_name/repo')).toBeNull(); - }); - - it('should return null for string with no slash', () => { - expect(extractOrgFromSource('anthropic')).toBeNull(); - }); }); - describe('edge cases', () => { - it('should handle whitespace around input', () => { - expect(extractOrgFromSource(' github:anthropic/repo ')).toBe('anthropic'); - }); - - it('should handle multiple # in branch name', () => { - expect(extractOrgFromSource('github:anthropic/repo#branch#with#hashes')).toBe('anthropic'); - }); - - it('should handle empty branch after #', () => { - expect(extractOrgFromSource('github:anthropic/repo#')).toBe('anthropic'); - }); - - it('should be case-sensitive for org names', () => { - expect(extractOrgFromSource('github:Anthropic/repo')).toBe('Anthropic'); - expect(extractOrgFromSource('github:ANTHROPIC/repo')).toBe('ANTHROPIC'); - }); - - it('should handle long org names', () => { - const longOrg = 'a'.repeat(39); - expect(extractOrgFromSource(`github:${longOrg}/repo`)).toBe(longOrg); - }); - - it('should handle org names with numbers', () => { - expect(extractOrgFromSource('github:123org/repo')).toBe('123org'); - expect(extractOrgFromSource('github:org456/repo')).toBe('org456'); - }); + it('should handle whitespace and preserve case', () => { + expect(extractOrgFromSource(' github:anthropic/repo ')).toBe('anthropic'); + expect(extractOrgFromSource('github:Anthropic/repo')).toBe('Anthropic'); }); });