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 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: 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..599b8db8 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 && { skillNameMap }) }, ); copyResults.push(...results); } @@ -689,6 +701,88 @@ 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]; + if (!skill || !entry) continue; + 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 +910,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..eae0e234 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 && { skillNameMap }) }), + copyHooks(pluginPath, workspacePath, client, baseOptions), + copyAgents(pluginPath, workspacePath, client, baseOptions), ]); return [...commandResults, ...skillResults, ...hookResults, ...agentResults]; 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/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/src/utils/source-parser.ts b/src/utils/source-parser.ts new file mode 100644 index 00000000..7b9be1a8 --- /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?.[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/integration/skill-duplicate-handling.test.ts b/tests/integration/skill-duplicate-handling.test.ts new file mode 100644 index 00000000..eae1abc0 --- /dev/null +++ b/tests/integration/skill-duplicate-handling.test.ts @@ -0,0 +1,167 @@ +/** + * Integration tests for duplicate skill handling in the full sync flow. + */ + +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 { CONFIG_DIR, WORKSPACE_CONFIG_FILE } from '../../src/constants.js'; +import { getShortId } from '../../src/utils/hash.js'; + +describe('Skill duplicate handling', () => { + let testDir: string; + + beforeEach(async () => { + testDir = await mkdtemp(join(tmpdir(), 'allagents-skill-dup-')); + }); + + afterEach(async () => { + await rm(testDir, { recursive: true, force: true }); + }); + + 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'), `---\nname: ${skill}\ndescription: Test skill\n---\n# ${skill}`); + } + } + + async function createWorkspaceConfig(plugins: string[]): Promise { + await mkdir(join(testDir, CONFIG_DIR), { recursive: true }); + await writeFile( + join(testDir, CONFIG_DIR, WORKSPACE_CONFIG_FILE), + `repositories: []\nplugins:\n${plugins.map((p) => ` - ${p}`).join('\n')}\nclients:\n - claude`, + ); + } + + async function getSyncedSkills(): Promise { + const dir = join(testDir, '.claude', 'skills'); + return existsSync(dir) ? readdir(dir) : []; + } + + 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']); + + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + const skills = await getSyncedSkills(); + expect(skills.sort()).toEqual(['skill-a', 'skill-b']); + }); + + 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']); + + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + const skills = await getSyncedSkills(); + expect(skills.sort()).toEqual(['alpha_common', 'beta_common']); + }); + + 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']); + + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + + 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'); + }); + + 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`); + }); + + 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`); + }); + + 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 syncWorkspace(testDir); + let skills = await getSyncedSkills(); + expect(skills).toContain('plugin-a_coding'); + expect(skills).toContain('plugin-b_coding'); + + // Remove conflict + await createWorkspaceConfig(['./plugin-a']); + await syncWorkspace(testDir); + + skills = await getSyncedSkills(); + expect(skills).toContain('coding'); + expect(skills).not.toContain('plugin-a_coding'); + }); + + 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/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'); + }); +}); 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)); + }); +}); 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', + ); + }); +}); diff --git a/tests/unit/utils/source-parser.test.ts b/tests/unit/utils/source-parser.test.ts new file mode 100644 index 00000000..928b4dd6 --- /dev/null +++ b/tests/unit/utils/source-parser.test.ts @@ -0,0 +1,50 @@ +import { describe, it, expect } from 'bun:test'; +import { extractOrgFromSource } from '../../../src/utils/source-parser.js'; + +describe('extractOrgFromSource', () => { + describe('GitHub formats', () => { + it('should extract org from github: and gh: prefix formats', () => { + expect(extractOrgFromSource('github:anthropic/repo')).toBe('anthropic'); + expect(extractOrgFromSource('github:anthropic/repo#main')).toBe('anthropic'); + expect(extractOrgFromSource('gh:my-org/repo')).toBe('my-org'); + expect(extractOrgFromSource('gh:my-org/repo#branch')).toBe('my-org'); + }); + + 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 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 local paths', () => { + expect(extractOrgFromSource('./local/path')).toBeNull(); + expect(extractOrgFromSource('../parent/path')).toBeNull(); + expect(extractOrgFromSource('/home/user/plugins')).toBeNull(); + expect(extractOrgFromSource('C:\\Users\\test\\plugins')).toBeNull(); + expect(extractOrgFromSource('.')).toBeNull(); + }); + }); + + describe('invalid input - should return null', () => { + it('should return null for invalid input', () => { + expect(extractOrgFromSource('')).toBeNull(); + expect(extractOrgFromSource(' ')).toBeNull(); + expect(extractOrgFromSource('github:')).toBeNull(); + expect(extractOrgFromSource('github:/repo')).toBeNull(); + expect(extractOrgFromSource('github:-invalid/repo')).toBeNull(); + expect(extractOrgFromSource('github:in--valid/repo')).toBeNull(); + }); + }); + + it('should handle whitespace and preserve case', () => { + expect(extractOrgFromSource(' github:anthropic/repo ')).toBe('anthropic'); + expect(extractOrgFromSource('github:Anthropic/repo')).toBe('Anthropic'); + }); +});