-
Notifications
You must be signed in to change notification settings - Fork 1
refactor: make SKILLS_REPOS configurable via SKILLS.txt #59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThe pull request externalize skill repository definitions into a separate SKILLS.txt file, refactoring the Makefile to read repositories dynamically. Additionally, it replaces symlink-based dot-directory setup with rsync-based synchronization and simplifies ruler.toml configuration by removing agent-specific blocks. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR aims to make skill repositories configurable by moving the SKILLS_REPOS list from hardcoded Makefile variables to an external SKILLS.txt file. However, the PR includes multiple unrelated changes that significantly expand its scope beyond the stated objective.
Changes:
- Created SKILLS.txt file with skill repository list (including one new repository not in original)
- Updated Makefile to load SKILL_REPOS from SKILLS.txt dynamically
- Changed ruler-dotdirs-link target to ruler-dotdirs-sync, replacing symlink creation with rsync copying
- Removed multiple agent configurations from ruler.toml and reordered default_agents list
- Changed gitignore.enabled from false to true in ruler.toml
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| SKILLS.txt | New file containing skill repository list (5 repos including 1 new addition) |
| Makefile | Modified to load SKILL_REPOS from SKILLS.txt; refactored ruler-dotdirs-link to ruler-dotdirs-sync with behavioral changes |
| .ruler/ruler.toml | Removed multiple agent configurations, reordered default_agents, and enabled gitignore |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| better-auth/skills \ | ||
| trailofbits/skills \ | ||
| vercel-labs/agent-skills | ||
| SKILLS_FILE := $(dir $(lastword $(MAKEFILE_LIST)))SKILLS.txt |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SKILLS_FILE variable uses a complex expression "$(dir
| SKILLS_FILE := $(dir $(lastword $(MAKEFILE_LIST)))SKILLS.txt | |
| SKILLS_FILE := SKILLS.txt |
| trailofbits/skills \ | ||
| vercel-labs/agent-skills | ||
| SKILLS_FILE := $(dir $(lastword $(MAKEFILE_LIST)))SKILLS.txt | ||
| SKILL_REPOS := $(shell cat $(SKILLS_FILE) 2>/dev/null) |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While the command includes "2>/dev/null" to suppress errors, if SKILLS.txt is missing or empty, SKILL_REPOS will be empty and the skills-install target will silently do nothing. Consider adding validation in the skills-install target to check if SKILL_REPOS is empty and provide a helpful error message.
| SKILL_REPOS := $(shell cat $(SKILLS_FILE) 2>/dev/null) | |
| SKILL_REPOS := $(shell cat $(SKILLS_FILE) 2>/dev/null) | |
| ifeq ($(strip $(SKILL_REPOS)),) | |
| $(warning SKILLS.txt is missing or empty; no skills will be installed by the 'skills-install' target.) | |
| endif |
| @@ -0,0 +1,5 @@ | |||
| anthropics/claude-plugins-official | |||
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SKILLS.txt file lacks documentation explaining its format and purpose. Consider adding a comment at the top of the file explaining that each line should contain a GitHub repository in the format "owner/repo" and that these repositories will be installed as skills.
| @make mcp-sync | ||
| @make ruler-apply-global | ||
| @make ruler-dotdirs-link | ||
| @make ruler-dotdirs-sync |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The call to "ruler-dotdirs-link" has been changed to "ruler-dotdirs-sync" in the sync target. While this matches the renamed target below, this behavioral change (from symlink creation to rsync copying) is unrelated to making SKILLS_REPOS configurable and should be in a separate PR.
| @@ -85,54 +85,6 @@ output_path = ".goosehints" | |||
| [agents.jules] | |||
| enabled = true | |||
|
|
|||
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The removal of multiple agent configurations (pi, roo, antigravity, amazonqcli, qwen, zed, trae, warp, kiro, firebender, agentsmd, mistral) is unrelated to making SKILLS_REPOS configurable. This substantial configuration change should be in a separate PR with proper justification and documentation.
| [agents.pi] | |
| enabled = true | |
| [agents.roo] | |
| enabled = true | |
| [agents.antigravity] | |
| enabled = true | |
| [agents.amazonqcli] | |
| enabled = true | |
| [agents.qwen] | |
| enabled = true | |
| [agents.zed] | |
| enabled = true | |
| [agents.trae] | |
| enabled = true | |
| [agents.warp] | |
| enabled = true | |
| [agents.kiro] | |
| enabled = true | |
| [agents.firebender] | |
| enabled = true | |
| [agents.agentsmd] | |
| enabled = true | |
| [agents.mistral] | |
| enabled = true |
| [gitignore] | ||
| # Enable/disable automatic .gitignore updates (default: true) | ||
| enabled = false | ||
| enabled = true |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change from "enabled = false" to "enabled = true" for gitignore configuration is unrelated to making SKILLS_REPOS configurable. This behavioral change should be in a separate PR to keep changes focused and traceable.
| enabled = true | |
| enabled = false |
| @@ -0,0 +1,5 @@ | |||
| anthropics/claude-plugins-official | |||
| better-auth/skills | |||
| subsy/ralph-tui | |||
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SKILLS.txt file includes a new repository "subsy/ralph-tui" that was not present in the original SKILL_REPOS list in the Makefile. This appears to be an unrelated addition that should not be part of this refactoring PR. If this repository should be added, it should be done in a separate PR with appropriate justification and testing.
| subsy/ralph-tui |
| .PHONY: ruler-dotdirs-sync | ||
| ruler-dotdirs-sync: ## Sync repo dot directories to $HOME equivalents. | ||
| @bash -c 'set -e; \ | ||
| root="$$(pwd)"; \ | ||
| dirs="$(DOTDIRS)"; \ | ||
| for d in $$dirs; do \ | ||
| src="$$root/$$d"; \ | ||
| target="$$HOME/$$d"; \ | ||
| link="$$root/$$d"; \ | ||
| if [ -e "$$link" ] && [ ! -L "$$link" ]; then \ | ||
| echo "Skipping $$link (exists and is not a symlink)" >&2; \ | ||
| continue; \ | ||
| fi; \ | ||
| if [ ! -e "$$target" ]; then \ | ||
| if [ -d "$$src" ]; then \ | ||
| mkdir -p "$$target"; \ | ||
| rsync -a "$$src/" "$$target/"; \ | ||
| echo "Synced $$src → $$target"; \ | ||
| fi; \ | ||
| ln -sfn "$$target" "$$link"; \ | ||
| done' |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The renaming and refactoring of "ruler-dotdirs-link" to "ruler-dotdirs-sync" changes the functionality from creating symlinks to using rsync for copying files. This is a significant behavioral change that appears unrelated to the PR's stated objective of making SKILLS_REPOS configurable. This change should be in a separate PR with its own testing and justification, as it affects how dot directories are managed.
|
|
||
| # Default agents to use when --agents is not specified | ||
| default_agents = ["copilot", "claude", "cursor", "codex", "aider", "amp", "windsurf", "cline", "gemini-cli", "kilocode", "firebase", "crush", "openhands", "junie", "augmentcode", "opencode", "goose", "jules", "pi", "roo", "antigravity", "amazonqcli", "qwen", "zed", "trae", "warp", "kiro", "firebender", "agentsmd", "mistral"] | ||
| default_agents = ["copilot", "claude", "codex", "cursor", "windsurf", "cline", "aider", "firebase", "openhands", "gemini-cli", "jules", "junie", "augmentcode", "kilocode", "opencode", "goose", "crush", "amp"] |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default_agents list has been significantly reordered and modified. This change appears unrelated to the PR's stated objective of making SKILLS_REPOS configurable via SKILLS.txt. Changes to agent configuration should be in a separate PR to maintain clear change history and facilitate easier review and rollback if needed.
| default_agents = ["copilot", "claude", "codex", "cursor", "windsurf", "cline", "aider", "firebase", "openhands", "gemini-cli", "jules", "junie", "augmentcode", "kilocode", "opencode", "goose", "crush", "amp"] | |
| default_agents = ["amp", "copilot", "claude", "codex", "cursor", "windsurf", "cline", "aider", "kilocode", "firebase", "gemini-cli", "crush", "openhands", "junie", "augmentcode", "opencode", "goose", "jules"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 3 files
Changes
Technical Details
Testing
Generated with opencode by claude-3.5-sonnet
Summary by cubic
Skill repositories are now configurable via SKILLS.txt, so you can add or update skills without touching the Makefile. Dotdir syncing uses rsync, and Ruler defaults were simplified with .gitignore updates enabled.
Refactors
Migration
Written for commit 1a57d1a. Summary will update on new commits.