fix(install): migrate skills from agents/skills/ to skills/#18
fix(install): migrate skills from agents/skills/ to skills/#18maystudios wants to merge 1 commit intomainfrom
Conversation
Move MAXSIM skill install destination from .claude/agents/skills/ to .claude/skills/ (Claude Code's native skills directory). Extract shared BUILT_IN_SKILLS constant and removeBuiltInSkills helper to eliminate 4x duplication. Add legacy agents/skills/ cleanup during install and uninstall for seamless upgrades. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Migrates MAXSIM’s skill installation and all template references from the legacy agents/skills/ location to Claude Code’s native skills/ directory, while consolidating built-in skill cleanup logic into shared helpers and extending install/uninstall cleanup for backward compatibility.
Changes:
- Update workflow/agent templates to reference
skills/instead of.agents/skills/. - Install/uninstall now manage skills under
skills/and also clean up legacyagents/skills/. - Extract
BUILT_IN_SKILLSandremoveBuiltInSkills()intopackages/cli/src/install/shared.tsand update manifest paths accordingly.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/workflows/quick.md | Update “Project skills” references from .agents/skills/ to skills/. |
| templates/workflows/plan-phase.md | Update “Project skills” references from .agents/skills/ to skills/. |
| templates/workflows/execute-phase.md | Update “Project skills” references from .agents/skills/ to skills/. |
| templates/agents/maxsim-planner.md | Update project-skill lookup paths and skill table links to skills/.... |
| templates/agents/maxsim-plan-checker.md | Update project-skill lookup paths and skill table links to skills/.... |
| templates/agents/maxsim-phase-researcher.md | Update project-skill lookup paths and skill table links to skills/.... |
| templates/agents/maxsim-executor.md | Update project-skill lookup paths and skill table links to skills/.... |
| templates/agents/maxsim-debugger.md | Update skill table links to skills/.... |
| packages/cli/src/install/uninstall.ts | Add removal of MAXSIM skills from skills/ and legacy agents/skills/. |
| packages/cli/src/install/shared.ts | Introduce shared built-in skill list and helper to remove them. |
| packages/cli/src/install/manifest.ts | Record installed skills under skills/ in the manifest. |
| packages/cli/src/install/index.ts | Install skills into skills/ and use shared cleanup helper. |
| packages/cli/src/install/hooks.ts | Add orphan cleanup for legacy agents/skills/ via shared helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 'batch-execution', | ||
| 'subagent-driven-development', | ||
| 'writing-plans', |
There was a problem hiding this comment.
BUILT_IN_SKILLS includes entries that don't exist in the bundled templates (templates/skills/ currently has: code-review, memory-management, simplify, systematic-debugging, tdd, using-maxsim, verification-before-completion). Keeping extra names here can lead to unintended deletion of user-created skills with those names during install/uninstall/cleanup. Update the constant to exactly match the skills MAXSIM actually installs (or derive the list from the templates directory at build/runtime).
| 'batch-execution', | |
| 'subagent-driven-development', | |
| 'writing-plans', |
| for (const skill of BUILT_IN_SKILLS) { | ||
| const skillDir = path.join(skillsParentDir, skill); | ||
| if (fs.existsSync(skillDir)) { | ||
| fs.rmSync(skillDir, { recursive: true }); |
There was a problem hiding this comment.
removeBuiltInSkills() uses fs.rmSync(..., { recursive: true }) even though this module already provides safeRmDir() specifically to handle cross-platform removal issues (e.g., Windows EPERM/read-only files). Consider using the same removal helper here (or fs.rmSync with the options needed for Windows) so uninstall/cleanup doesn't fail on Windows when deleting skill directories.
| fs.rmSync(skillDir, { recursive: true }); | |
| safeRmDir(skillDir); |
| // Include skills in manifest (skills/<skill-name>/*) | ||
| const skillsManifestDir = path.join(configDir, 'skills'); | ||
| if (fs.existsSync(skillsManifestDir)) { | ||
| const skillHashes = generateManifest(skillsManifestDir); | ||
| for (const [rel, hash] of Object.entries(skillHashes)) { | ||
| manifest.files['agents/skills/' + rel] = hash; | ||
| manifest.files['skills/' + rel] = hash; | ||
| } |
There was a problem hiding this comment.
writeManifest() now hashes everything under configDir/skills and records it as MAXSIM-managed. Since skills/ is a shared, user-editable directory, this will cause saveLocalPatches() to back up (and report) user skills as “modified MAXSIM files” on the next install. Limit manifest entries to MAXSIM-owned skill directories (same set you remove/overwrite during install) rather than the entire skills/ tree.
| const installedSkillDirs = fs.readdirSync(skillsDest, { withFileTypes: true }) | ||
| .filter(e => e.isDirectory()).length; | ||
| if (installedSkillDirs > 0) { | ||
| spinner.succeed(chalk.green('\u2713') + ` Installed ${installedSkillDirs} skills to agents/skills/`); | ||
| spinner.succeed(chalk.green('\u2713') + ` Installed ${installedSkillDirs} skills to skills/`); | ||
| } else { | ||
| spinner.fail('Failed to install skills'); | ||
| failures.push('agents/skills'); | ||
| failures.push('skills'); |
There was a problem hiding this comment.
installedSkillDirs counts all subdirectories under the shared skills/ directory, including pre-existing user skills. This makes the “Installed X skills” message inaccurate and can mask an install problem (e.g., if MAXSIM skills failed to copy but user skills already exist, the count is still > 0). Consider validating/counting only the MAXSIM-installed skills (the built-in list) when reporting success/failure.
Summary
.claude/agents/skills/to.claude/skills/(Claude Code's native skills directory)BUILT_IN_SKILLSconstant andremoveBuiltInSkills()helper intoshared.tsto eliminate 4x duplication across install, uninstall, and hooks modulesagents/skills/cleanup during both install (orphan cleanup) and uninstall for seamless upgrades from older versions.agents/skills/toskills/skills/instead ofagents/skills/Test plan
npx vitest run)npm run build)~/.claude/skills/~/.claude/agents/skills/skills/and legacyagents/skills/🤖 Generated with Claude Code