docs: Clarify skill naming and trigger behavior for custom skills#348
docs: Clarify skill naming and trigger behavior for custom skills#348
Conversation
- Add two options for customizing code review: replace or supplement - Document that multiple skills with same trigger but different names coexist - Explain skill precedence is by name, not by trigger - Add recommended approach (Option 2) for adding supplementary guidelines - Fix path inconsistency (.agents/ vs .openhands/) - Add code example and tips for using multiple skills with same trigger This addresses confusion where users thought they had to replace the default skill when they actually wanted to add project-specific guidelines alongside it. Co-authored-by: openhands <openhands@all-hands.dev>
- Use custom-codereview-guide.md as the recommended filename - Note that using code-review.md will override the default skill - Remove separate sections for replace/supplement options Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
Taste Rating: 🟡 Acceptable - Solves a real problem (skill name vs trigger confusion) but needs clarity improvements.
Core Insight is Solid: Skill deduplication by NAME not TRIGGER is the right mental model to document.
Critical Issues to Address
🟠 Path Inconsistency (pr-review.mdx, line 13)
The new example uses .agents/skills/custom-codereview-guide.md but the old version mentioned .openhands/skills/code-review.md. Which is canonical?
Required: Explicitly document which path(s) are supported, precedence if both exist, and whether this is a migration.
🟠 Vague Merge Behavior (pr-review.mdx, lines 84-87)
"The agent receives guidelines from both skills" - HOW exactly? Be technically precise:
- Do they concatenate in the system prompt?
- What order (default first, then custom)?
- If guidelines conflict, which wins?
- Raw concatenation or smart merging?
🟠 Verify Reference Exists (pr-review.mdx, line 99)
Does software-agent-sdk/.agents/skills/custom-codereview-guide.md actually exist? Broken references hurt credibility. Create the file first or use a different example.
Suggestions
🟡 Add Migration Guidance (pr-review.mdx, line 18)
Users who followed old docs may have .openhands/skills/code-review.md. Add:
Migrating: If you created
code-review.mdto override, rename tocustom-codereview-guide.mdto supplement instead.
🟡 Improve Example Clarity (pr-review.mdx, lines 54-57)
The example doesn't clearly show SUPPLEMENTARY nature. Add:
**Note**: These rules supplement the default code-review skill.🟡 Document Merge Behavior (skill.mdx, lines 611-614)
After precedence explanation, add:
Skill Activation: When multiple skills share a trigger, all are loaded. Content concatenates (public first, then explicit).
What's Good
🟢 The code example in skill.mdx (lines 616-628) is clear with helpful comments.
Verdict: ✅ Worth merging after addressing the 3 critical clarifications (path convention, merge behavior, reference validity).
Key Insight: You've identified the right conceptual model (name-based deduplication, trigger-based activation), but the documentation needs technical precision to match that clarity.
|
@OpenHands pls fix #348 (review) |
|
I'm on it! xingyaoww can track my progress at all-hands.dev |
- Add note about supported skill paths (.agents/skills/ recommended, .openhands/skills/ supported) with link to precedence docs - Document merge behavior with technical precision: content concatenates (public first, then custom), no smart merging - Add migration guidance for users who created code-review.md to override - Add note clarifying the example rules supplement the default skill - Add Tip component in skill.mdx explaining skill activation behavior Co-authored-by: openhands <openhands@all-hands.dev>
SummaryI addressed all feedback from PR review #3821694947 on PR #348. The changes have been pushed in commit Checklist of Review Feedback Addressed:Critical Issues:
Suggestions:
Files Changed:
The changes are concise and focused solely on addressing the review feedback—no extraneous modifications were made. |
Co-authored-by: Engel Nyst <engel.nyst@gmail.com>
4 participants
Summary
This PR clarifies the documentation around skill naming and trigger behavior, specifically addressing confusion about how custom code review skills interact with the default public skills.
Problem
The current documentation suggests creating a skill with
name: code-reviewand trigger/codereview, which unintentionally replaces the default public skill entirely. Users who want to add project-specific guidelines alongside (not instead of) the default skill weren't aware of the alternative approach.Solution
Updated documentation to clearly explain two options:
1. Replace the default skill (Option 1)
Use
name: code-reviewto completely replace the public skill with your own.2. Add supplementary guidelines - Recommended (Option 2)
Use a different name like
my-project-reviewwith the same/codereviewtrigger. Both skills will be activated together, giving you the default review guidelines PLUS your project-specific additions.Key Technical Insight
Skill deduplication is by NAME, not by trigger. Multiple skills with different names but the same trigger can coexist and will ALL be triggered when the keyword matches.
Changes
sdk/guides/github-workflows/pr-review.mdx:.agents/and.openhands/)sdk/guides/skill.mdx:@xingyaoww can click here to continue refining the PR