diff --git a/Packages/src/Cli~/src/__tests__/cli-e2e.test.ts b/Packages/src/Cli~/src/__tests__/cli-e2e.test.ts index 2c20d75c8..0344c3f9d 100644 --- a/Packages/src/Cli~/src/__tests__/cli-e2e.test.ts +++ b/Packages/src/Cli~/src/__tests__/cli-e2e.test.ts @@ -669,6 +669,7 @@ describe('CLI E2E Tests (requires running Unity)', () => { }); it('should install skills for claude target', () => { + // Verifies default installs land where Claude Code can discover skills. // First uninstall to ensure clean state runCli('skills uninstall --claude'); @@ -677,7 +678,6 @@ describe('CLI E2E Tests (requires running Unity)', () => { UNITY_PROJECT_ROOT, '.claude', 'skills', - 'unity-cli-loop', 'uloop-compile', 'SKILL.md', ); @@ -687,7 +687,8 @@ describe('CLI E2E Tests (requires running Unity)', () => { expect(existsSync(installedSkillPath)).toBe(true); }); - it('should install skills directly under skills when grouping is disabled', () => { + it('should install skills directly under skills when flat flag is provided', () => { + // Verifies the explicit flat flag remains accepted for discoverable installs. runCli('skills uninstall --claude'); const { stdout, exitCode } = runCli('skills install --claude --flat'); diff --git a/Packages/src/Cli~/src/__tests__/skills-command.test.ts b/Packages/src/Cli~/src/__tests__/skills-command.test.ts new file mode 100644 index 000000000..fbc0f3e29 --- /dev/null +++ b/Packages/src/Cli~/src/__tests__/skills-command.test.ts @@ -0,0 +1,79 @@ +interface MockInstallResult { + installed: number; + updated: number; + skipped: number; + bundledCount: number; + projectCount: number; + deprecatedRemoved: number; +} + +interface MockUninstallResult { + removed: number; + notFound: number; +} + +const mockGetAllSkillStatuses = jest.fn(); +const mockInstallAllSkills = jest.fn(); +const mockUninstallAllSkills = jest.fn(); +const mockGetInstallDir = jest.fn(); +const mockGetTotalSkillCount = jest.fn(); + +jest.mock('../skills/skills-manager.js', () => ({ + DEFAULT_GROUP_MANAGED_SKILLS: false, + getAllSkillStatuses: (target: unknown, global: boolean, groupManagedSkills: boolean): unknown[] => + mockGetAllSkillStatuses(target, global, groupManagedSkills), + installAllSkills: ( + target: unknown, + global: boolean, + groupManagedSkills: boolean, + ): MockInstallResult => mockInstallAllSkills(target, global, groupManagedSkills), + uninstallAllSkills: ( + target: unknown, + global: boolean, + groupManagedSkills: boolean, + ): MockUninstallResult => mockUninstallAllSkills(target, global, groupManagedSkills), + getInstallDir: (target: unknown, global: boolean, groupManagedSkills: boolean): string => + mockGetInstallDir(target, global, groupManagedSkills), + getTotalSkillCount: (): number => mockGetTotalSkillCount(), +})); + +import { Command } from 'commander'; +import { registerSkillsCommand } from '../skills/skills-command.js'; + +describe('skills command', () => { + let consoleLogSpy: jest.SpyInstance; + + beforeEach(() => { + jest.clearAllMocks(); + consoleLogSpy = jest.spyOn(console, 'log').mockImplementation(() => undefined); + mockInstallAllSkills.mockReturnValue({ + installed: 1, + updated: 0, + skipped: 0, + bundledCount: 1, + projectCount: 0, + deprecatedRemoved: 0, + }); + mockUninstallAllSkills.mockReturnValue({ removed: 1, notFound: 0 }); + mockGetInstallDir.mockReturnValue('/home/user/.claude/skills'); + mockGetTotalSkillCount.mockReturnValue(1); + }); + + afterEach(() => { + consoleLogSpy.mockRestore(); + }); + + it('installs Claude Code skills into the discoverable flat layout by default', async () => { + // Verifies the default CLI path matches Claude Code skill discovery. + const program = new Command(); + registerSkillsCommand(program); + + await program.parseAsync(['node', 'uloop', 'skills', 'install', '--claude', '--global']); + + expect(mockInstallAllSkills).toHaveBeenCalledWith( + expect.objectContaining({ id: 'claude' }), + true, + false, + ); + }); +}); diff --git a/Packages/src/Cli~/src/__tests__/skills-manager.test.ts b/Packages/src/Cli~/src/__tests__/skills-manager.test.ts index a4e3fe682..b8be986a6 100644 --- a/Packages/src/Cli~/src/__tests__/skills-manager.test.ts +++ b/Packages/src/Cli~/src/__tests__/skills-manager.test.ts @@ -19,6 +19,7 @@ import { parseFrontmatter, removeDeprecatedSkillDirs, syncInstalledSkillDirectory, + uninstallAllSkills, } from '../skills/skills-manager.js'; import { getTargetConfig } from '../skills/target-config.js'; @@ -225,6 +226,13 @@ describe('skill install layout', () => { ); }); + it('should resolve the flat install directory by default', () => { + // Verifies default installs land where Claude Code can discover skills. + expect(getInstallDir(getTargetConfig('claude'), true)).toBe( + join(homedir(), '.claude', 'skills'), + ); + }); + it('should resolve the selected skill directory for uninstall operations', () => { const skillsRoot = createSkillsRoot(); @@ -376,7 +384,7 @@ describe('skill install layout', () => { process.chdir(projectRoot); - const skillsRoot = getInstallDir(getTargetConfig('claude'), false); + const skillsRoot = getInstallDir(getTargetConfig('claude'), false, true); syncInstalledSkillDirectory( join(skillsRoot, 'uloop-capture-tool'), 'SKILL.md', @@ -437,6 +445,28 @@ describe('skill install layout', () => { expect(existsSync(join(managedSkillsRoot, 'uloop-unity-search'))).toBe(false); }); + it('should remove an installed skill from both layouts by default', () => { + // Verifies default uninstall cleans up the previous grouped CLI layout. + const projectRoot = createUnityProjectRoot(); + const skillsRoot = join(projectRoot, '.claude', 'skills'); + const projectSkillDir = join(projectRoot, 'Assets', 'Test', 'Editor', 'CleanupTool', 'Skill'); + const skillName = 'uloop-cleanup-test'; + + writeSkill( + projectSkillDir, + ['---', `name: ${skillName}`, '---', '', '# Cleanup Test', ''].join('\n'), + ); + writeSkill(join(skillsRoot, skillName)); + writeSkill(join(skillsRoot, 'unity-cli-loop', skillName)); + process.chdir(projectRoot); + + const result = uninstallAllSkills(getTargetConfig('claude'), false); + + expect(result.removed).toBe(1); + expect(existsSync(join(skillsRoot, skillName))).toBe(false); + expect(existsSync(join(skillsRoot, 'unity-cli-loop', skillName))).toBe(false); + }); + it('should remove stale files when syncing an installed skill directory', () => { const skillsRoot = createSkillsRoot(); const skillDir = join(skillsRoot, 'unity-cli-loop', 'uloop-execute-dynamic-code'); diff --git a/Packages/src/Cli~/src/skills/skills-command.ts b/Packages/src/Cli~/src/skills/skills-command.ts index 14e94a301..afd53441a 100644 --- a/Packages/src/Cli~/src/skills/skills-command.ts +++ b/Packages/src/Cli~/src/skills/skills-command.ts @@ -12,6 +12,7 @@ import { uninstallAllSkills, getInstallDir, getTotalSkillCount, + DEFAULT_GROUP_MANAGED_SKILLS, } from './skills-manager.js'; import { TargetConfig, ALL_TARGET_IDS, getTargetConfig } from './target-config.js'; @@ -36,7 +37,7 @@ export function registerSkillsCommand(program: Command): void { .command('list') .description('List all uloop skills and their installation status') .option('-g, --global', 'Check global installation') - .option('--flat', 'Install directly under skills/ instead of skills/unity-cli-loop/') + .option('--flat', 'Use skills/ directly (default)') .option('--claude', 'Check Claude Code installation') .option('--codex', 'Check Codex CLI installation') .option('--cursor', 'Check Cursor installation') @@ -47,14 +48,14 @@ export function registerSkillsCommand(program: Command): void { .action((options: SkillsOptions) => { const targets = resolveTargets(options); const global = options.global ?? false; - listSkills(targets, global, !(options.flat ?? false)); + listSkills(targets, global, DEFAULT_GROUP_MANAGED_SKILLS); }); skillsCmd .command('install') .description('Install all uloop skills') .option('-g, --global', 'Install to global location') - .option('--flat', 'Install directly under skills/ instead of skills/unity-cli-loop/') + .option('--flat', 'Install directly under skills/ (default)') .option('--claude', 'Install to Claude Code') .option('--codex', 'Install to Codex CLI') .option('--cursor', 'Install to Cursor') @@ -68,14 +69,14 @@ export function registerSkillsCommand(program: Command): void { showTargetGuidance('install'); return; } - installSkills(targets, options.global ?? false, !(options.flat ?? false)); + installSkills(targets, options.global ?? false, DEFAULT_GROUP_MANAGED_SKILLS); }); skillsCmd .command('uninstall') .description('Uninstall all uloop skills') .option('-g, --global', 'Uninstall from global location') - .option('--flat', 'Uninstall skills installed directly under skills/') + .option('--flat', 'Uninstall skills installed directly under skills/ (default)') .option('--claude', 'Uninstall from Claude Code') .option('--codex', 'Uninstall from Codex CLI') .option('--cursor', 'Uninstall from Cursor') @@ -89,7 +90,7 @@ export function registerSkillsCommand(program: Command): void { showTargetGuidance('uninstall'); return; } - uninstallSkills(targets, options.global ?? false, !(options.flat ?? false)); + uninstallSkills(targets, options.global ?? false, DEFAULT_GROUP_MANAGED_SKILLS); }); } @@ -123,17 +124,17 @@ function showTargetGuidance(command: string): void { console.log(`\nPlease specify at least one target for '${command}':`); console.log(''); console.log('Available targets:'); - console.log(' --claude Claude Code (.claude/skills/unity-cli-loop/)'); - console.log(' --codex Codex CLI (.codex/skills/unity-cli-loop/)'); - console.log(' --cursor Cursor (.cursor/skills/unity-cli-loop/)'); - console.log(' --gemini Gemini CLI (.gemini/skills/unity-cli-loop/)'); - console.log(' --agents Other (.agents) (.agents/skills/unity-cli-loop/)'); - console.log(' --windsurf Windsurf (.agents/skills/unity-cli-loop/)'); - console.log(' --antigravity Antigravity (.agent/skills/unity-cli-loop/)'); + console.log(' --claude Claude Code (.claude/skills/)'); + console.log(' --codex Codex CLI (.codex/skills/)'); + console.log(' --cursor Cursor (.cursor/skills/)'); + console.log(' --gemini Gemini CLI (.gemini/skills/)'); + console.log(' --agents Other (.agents) (.agents/skills/)'); + console.log(' --windsurf Windsurf (.agents/skills/)'); + console.log(' --antigravity Antigravity (.agent/skills/)'); console.log(''); console.log('Options:'); console.log(' -g, --global Use global location'); - console.log(' --flat Use skills/ directly instead of skills/unity-cli-loop/'); + console.log(' --flat Use skills/ directly (default)'); console.log(''); console.log('Examples:'); console.log(` uloop skills ${command} --claude`); diff --git a/Packages/src/Cli~/src/skills/skills-manager.ts b/Packages/src/Cli~/src/skills/skills-manager.ts index b5d1f4f40..3234c6e99 100644 --- a/Packages/src/Cli~/src/skills/skills-manager.ts +++ b/Packages/src/Cli~/src/skills/skills-manager.ts @@ -56,6 +56,7 @@ const EXCLUDED_DIRS = new Set([ 'Skill', ]); const EXCLUDED_FILES = new Set(['.meta', '.DS_Store', '.gitkeep']); +export const DEFAULT_GROUP_MANAGED_SKILLS = false; class SkillsPathConstants { public static readonly PACKAGES_DIR = 'Packages'; public static readonly SRC_DIR = 'src'; @@ -268,7 +269,7 @@ function getSkillStatus( return 'installed'; } -/** @internal Move managed skills from the legacy flat layout into the namespaced install root. */ +/** @internal Move top-level managed skills into the namespaced install root when grouping is requested. */ export function migrateLegacyManagedSkills( baseDir: string, managedSkillDirNames: readonly string[], @@ -625,7 +626,7 @@ function resolveSkillSearchRootCandidate(candidate: string): string { export function getAllSkillStatuses( target: TargetConfig, global: boolean, - groupManagedSkills: boolean = true, + groupManagedSkills: boolean = DEFAULT_GROUP_MANAGED_SKILLS, ): SkillInfo[] { const allSkills = collectAllSkills(); return allSkills.map((skill) => ({ @@ -755,7 +756,7 @@ interface InstallResult { export function installAllSkills( target: TargetConfig, global: boolean, - groupManagedSkills: boolean = true, + groupManagedSkills: boolean = DEFAULT_GROUP_MANAGED_SKILLS, ): InstallResult { const result: InstallResult = { installed: 0, @@ -826,7 +827,7 @@ interface UninstallResult { export function uninstallAllSkills( target: TargetConfig, global: boolean, - groupManagedSkills: boolean = true, + groupManagedSkills: boolean = DEFAULT_GROUP_MANAGED_SKILLS, ): UninstallResult { const result: UninstallResult = { removed: 0, notFound: 0 }; @@ -835,7 +836,11 @@ export function uninstallAllSkills( const allSkills = collectAllSkills(); for (const skill of allSkills) { - if (uninstallSkill(skill, target, global, groupManagedSkills)) { + const removed = groupManagedSkills + ? uninstallSkill(skill, target, global, groupManagedSkills) + : uninstallSkillFromAllLayouts(skill, target, global); + + if (removed) { result.removed++; } else { result.notFound++; @@ -848,7 +853,7 @@ export function uninstallAllSkills( export function getInstallDir( target: TargetConfig, global: boolean, - groupManagedSkills: boolean = true, + groupManagedSkills: boolean = DEFAULT_GROUP_MANAGED_SKILLS, ): string { const baseDir = getSkillsBaseDir(target, global); return groupManagedSkills ? getManagedSkillsDir(baseDir) : baseDir;