fix(skill): dedupe symlinked external skill paths#17294
fix(skill): dedupe symlinked external skill paths#17294notzenco wants to merge 1 commit intoanomalyco:devfrom
Conversation
|
The following comment was made by an LLM, it may be inaccurate: Potential Duplicate Found:
Why they might be related: |
There was a problem hiding this comment.
Pull request overview
This PR improves skill discovery in packages/opencode by deduplicating skills that are reachable via multiple paths (e.g., symlinked .claude/skills and .agents/skills entries), ensuring Skill.all() / Skill.dirs() don’t double-count the same underlying skill.
Changes:
- Canonicalize discovered skill file paths via
Filesystem.resolve()and dedupe via aseenset. - Use canonical paths for
locationand directory tracking (Skill.dirs()). - Add a regression test covering symlinked skill directories deduplication.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/opencode/src/skill/skill.ts | Canonicalizes skill paths and deduplicates loading/dirs by resolved path. |
| packages/opencode/test/skill/skill.test.ts | Adds test ensuring symlinked skill dirs are deduped to one skill/dir. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const addSkill = async (match: string) => { | ||
| const md = await ConfigMarkdown.parse(match).catch((err) => { | ||
| const file = Filesystem.resolve(match) | ||
| if (seen.has(file)) return | ||
| seen.add(file) | ||
|
|
||
| const md = await ConfigMarkdown.parse(file).catch((err) => { |
Issue for this PR
Closes #17219
Type of change
What does this PR do?
When the same external skill is discovered through both a real path and a symlinked path, we were treating them as separate entries while building the skill directory allowlist. That produced duplicate
external_directoryrules even though both paths pointed at the same physical skill.This change resolves each discovered skill to its canonical path before storing it, skips repeated physical files, and stores canonical skill directories for the permission allowlist. That removes duplicate rules for symlinked skill installs while keeping distinct physical skills unchanged.
How did you verify your code works?
.claude/skillsentry pointing at.agents/skillsand checks thatSkill.all()andSkill.dirs()only report one canonical locationbun test test/skill/skill.test.ts test/tool/skill.test.tsbun typecheckScreenshots / recordings
N/A
Checklist