feat(skills): manage provider skills from shared .skills#49
Conversation
📝 WalkthroughWalkthroughThis PR adds a Makefile ChangesDevelopment Skills System
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes a configuration file, updates .gitignore to refine Claude-related rules and include new patterns, and introduces a skills target in the Makefile to automate symlinking for AI provider skills. Feedback was provided to improve the robustness of the Makefile script by adding error handling with set -e and optimizing directory creation within the loop.
There was a problem hiding this comment.
Pull request overview
Adds shared, provider-agnostic “skills” management by introducing a .skills/ source-of-truth and automating linking into provider-specific directories, while cleaning up obsolete Claude hook configuration.
Changes:
- Add a
skillsMakefile target and run it as part ofmake dev. - Add initial shared skills under
.skills/(web-fetch, humanizer-en, humanizer-ko). - Simplify
.gitignorerules for.claude/and remove obsolete.claude/hooks/peon-ping/config.json.
Reviewed changes
Copilot reviewed 2 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Makefile | Adds skills target to symlink shared skills into provider directories and wires it into dev. |
| .skills/web-fetch/SKILL.md | Adds a new shared “web-fetch” skill document. |
| .skills/humanizer-ko/SKILL.md | Adds a new shared Korean “humanizer” skill document. |
| .skills/humanizer-en/SKILL.md | Adds a new shared English “humanizer” skill document. |
| .gitignore | Updates ignore/include rules for .claude/ and moves .remember ignore to a global section. |
| .claude/hooks/peon-ping/config.json | Removes obsolete hook configuration file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f36995008c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Makefile (1)
69-73: ⚡ Quick winMake symlink target path robust for custom provider directories.
On Line 73, the link target is hardcoded to
../.skills, which only works whenprovider_diris a direct child of repo root. IfSKILL_PROVIDER_DIRScontains nested paths, links resolve incorrectly.Proposed fix
`@for` provider_dir in $(or $(SKILL_PROVIDER_DIRS),.agents .claude); do \ mkdir -p "$$provider_dir"; \ target="$$provider_dir/skills"; \ + link_src="$$(realpath --relative-to="$$provider_dir" .skills 2>/dev/null || python -c 'import os,sys; print(os.path.relpath(".skills", sys.argv[1]))' "$$provider_dir")"; \ if [ ! -e "$$target" ] && [ ! -L "$$target" ]; then \ - ln -s ../.skills "$$target"; \ + ln -s "$$link_src" "$$target"; \ fi; \ - if [ "$$(readlink "$$target" 2>/dev/null)" != "../.skills" ]; then \ + if [ "$$(readlink "$$target" 2>/dev/null)" != "$$link_src" ]; then \ echo "[skills] failed to link $$target"; \ exit 1; \ fi; \🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 69 - 73, The symlink creation uses a hardcoded relative target "../.skills" which breaks for nested SKILL_PROVIDER_DIRS; update the ln command in the Makefile loop that defines provider_dir and target to create an absolute symlink to the repo's .skills (e.g. ln -s "$(CURDIR)/.skills" "$$target") so the link always points to the correct .skills directory regardless of provider_dir depth.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Makefile`:
- Around line 69-73: The symlink creation uses a hardcoded relative target
"../.skills" which breaks for nested SKILL_PROVIDER_DIRS; update the ln command
in the Makefile loop that defines provider_dir and target to create an absolute
symlink to the repo's .skills (e.g. ln -s "$(CURDIR)/.skills" "$$target") so the
link always points to the correct .skills directory regardless of provider_dir
depth.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21c7edf6eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This pull request updates the project automation and configuration, primarily by adding a new
skillsMakefile target to streamline the management of shared skill directories for AI providers. It also removes the obsolete.claude/hooks/peon-ping/config.jsonconfiguration file.Enhancements to project automation:
skillstarget to theMakefilethat links each skill from.skills/into both.agents/skillsand.claude/skills, ensuring shared skills are available for different AI providers. This target also includes checks for the presence of skills and provides informative output. (Makefile, MakefileR64-R86)devtarget in theMakefileto include the newskillsstep, ensuring skill linking is part of the standard development setup. (Makefile, MakefileL1-R11)Configuration cleanup:
.claude/hooks/peon-ping/config.jsonfile, eliminating unused or obsolete configuration.<!--PR Title Format
Please ensure your PR title follows the Conventional Commits format:
():
Types: feat, fix, docs, refactor, test, chore, perf, ci, build, style, revert
Example scopes (recommendations only; any lowercase scope is accepted by CI): user, product, order, payment, auth, api, database, middleware, service, controller, model, docker, deps, config, github
Examples:
-->
Summary by CodeRabbit