fix skill installation paths#34
Conversation
- Updated installation paths for Claude and Cursor skills from `.claude/rules` and `.cursor/rules` to `.claude/skills` and `.cursor/skills` across various files including CLI completions, commands, and documentation. - Adjusted test cases to reflect the new installation paths for skills.
| ".cursor/rules", | ||
| ".opencode/rules", | ||
| ".codex/rules", | ||
| ]; |
There was a problem hiding this comment.
Nice that KNOWN_PROJECT_DIRS now includes both skills/ and rules/ paths so project removal cleans up legacy installs.
One gap though: removeGlobal above (lines 47-55) only removes the config entry, it never deletes files from disk. If someone previously ran corvus install --global-claude, those files still sit at ~/.claude/rules/<skill>/ and corvus remove --global won't touch them.
The PR description says removal cleans up both legacy and new paths across both scopes. Worth either adding filesystem cleanup to removeGlobal or adjusting the description to clarify this only applies to project-scoped removals.
There was a problem hiding this comment.
Good eye on KNOWN_PROJECT_DIRS! On removeGlobal, it actually does handle filesystem cleanup — lines 54–59 iterate over globalSkillInstallParents(), which already includes both the canonical path (/.claude/skills) and the legacy path (/.claude/rules), and calls rmSync on any matching skill directory it finds. So corvus remove --global will clean up files from both locations. The config entry remova happens right after
| "project-codex": ".codex/rules", | ||
| }; | ||
| projectConfigSet("install_dir", dirMap[target] ?? ".claude/rules"); | ||
| projectConfigSet("install_dir", dirMap[target] ?? ".claude/skills"); |
There was a problem hiding this comment.
Something to think about: users who already ran corvus install --claude have install_dir: ".claude/rules" saved in their project config. After this update the default changes to .claude/skills, but the stored value is not migrated.
If someone re-runs corvus install without a target flag, it reads the stale config and keeps installing to rules/. That could lead to a split where some skills live in rules/ and newer ones in skills/.
A one-time migration that rewrites install_dir when it matches a known legacy value (e.g. .claude/rules -> .claude/skills) would close the gap.
There was a problem hiding this comment.
This is already covered. migrateLegacyProjectInstallDirIfNeeded runs automatically on every corvus command via the preAction hook in index.ts. It reads install_dir from .corvusrc, and if it finds a known legacy value like .claude/rules, it renames the on-disk skill tree into .claude/skills and rewrites the stored install_dir in one shot. So the split-state scenario you described is prevented the stale config is corrected before any install logic runs.
| assert_includes targets, "CHANGELOG.md" | ||
| assert_includes targets, "skills/assistant/promptify/SKILL.md" | ||
| assert_includes targets, "skills/design/design-frontend/rules/layout-use-spacing-scale.md" | ||
| assert_includes targets, "skills/frontend/design-frontend/rules/layout-use-spacing-scale.md" |
There was a problem hiding this comment.
This fix is correct (design-frontend does live under skills/frontend/), but it is unrelated to the skills-vs-rules path change. Would be cleaner as its own commit so the history stays easy to bisect.
There was a problem hiding this comment.
Fair point this is a pre-existing path correction (skills/design/ → skills/frontend/) that's orthogonal to the skills vs rules change. It was bundled in to keep the test suite green in one pass, but agreed it would have been cleaner as a separate commit
…ort and dependency resolution
0x7067
left a comment
0x7067
left a comment
There was a problem hiding this comment.
@Menendez2004 can I merge this or are you still working on it?
sure, you can merge it, it's done |
4 participants
Problem
Corvus copies toolkit skills (folders with
SKILL.md, optionalrules/,references/, etc.) into a target directory chosen per product (Claude Code, Cursor, and others).Originally, Corvus used rules-style paths for Claude and Cursor:
.claude/rules/<skill-name>/~/.claude/rules/<skill-name>/.cursor/rules/<skill-name>/~/.cursor/rules/<skill-name>/Those directories are where Cursor Rules and Claude Code rules (instruction files scoped as rules) are expected to live. The products treat
skills/andrules/as different concepts:SKILL.mdat the root, loaded as skills in the UI and activation model..mdcor markdown) used as standing rules, not the same as the Agent Skills folder layout.Putting full skill trees under
…/rules/…meant installs looked and behaved like rules, not like installed skills. That blocked the expected “skill plugin” / Agent Skill experience and contradicted how both ecosystems document local skills (e.g. project skills under.claude/skills/,.cursor/skills/).Change
Corvus now targets Agent Skills locations for Claude Code and Cursor:
.claude/skills/<skill>/~/.claude/skills/<skill>/.cursor/skills/<skill>/~/.cursor/skills/<skill>/Other targets (OpenCode, Codex) still use each product’s documented
rules/layout where that is the appropriate integration surface.corvus removecontinues to clean up both legacy paths (…/rules/…) and the new…/skills/…paths so older installs are not left behind.Summary
The change was necessary so installed toolkit content is treated as Agent Skills in Claude Code and Cursor, matching product semantics and documentation, instead of incorrectly landing under rules directories.