docs: correct how the skills installer places files - #1643
Conversation
The README said the installer "writes each one's own convention" and listed `.claude/skills/`, `.agents/skills/` and `.windsurf/skills/` as peers, with "around seventy others". Running it shows otherwise: the skills are installed once into `.agents/skills/<name>` and symlinked from there into each detected assistant's directory. After a run with Claude Code present, `.claude/skills/melonjs` is a symlink to `../../.agents/skills/melonjs`, not a second copy. The tool reports the figure itself: "77 agents". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q8aCt5SwH5D5Bi7qKXi2X8
There was a problem hiding this comment.
🟡 Changes recommended
The updated README phrasing is ambiguous about the exact install path and it leaves a duplicate “Skills” install snippet in DOC_README.md with conflicting instructions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates the README’s npx skills add documentation to match the installer’s actual behavior (single shared install under .agents/skills/ with per-assistant symlinks) and replaces the vague coverage estimate with the tool-reported agent count.
Changes:
- Describe the shared install location and symlink-based wiring into assistant-specific directories.
- Replace “around seventy others” with the installer’s reported “77 agents” coverage figure.
File summaries
| File | Description |
|---|---|
| README.md | Corrects the skills installer behavior description (shared install + symlinks) and updates the stated coverage count. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Review feedback on the previous commit: `.agents/skills/` read as one install directory, when the installer creates one directory per skill (`.agents/skills/melonjs-physics` and 22 siblings) and symlinks each into the assistant's own directory. The coverage figure is reported by the tool, not by the location. DOC_README.md carried the same passage and still described the old behaviour, so the two files contradicted each other. Correct it there as well. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q8aCt5SwH5D5Bi7qKXi2X8
There was a problem hiding this comment.
🟢 Approval recommended
Documentation-only update appears consistent across both docs and aligns with the stated installer behavior, with only minor optional wording improvements noted.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
Review feedback: the installer's supported-assistant list grows, so a bare "77 agents" drifts. Keep the figure, since it is what the tool prints, but frame it as what it reported at the time of writing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q8aCt5SwH5D5Bi7qKXi2X8
There was a problem hiding this comment.
🟢 Approval recommended
The changes are documentation-only and consistently update both duplicated installer passages to match the described installer behavior.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
Current state, head
Markdown only, two files, so nothing else should move. Nothing outstanding from my side, it is yours to merge whenever you are happy with the wording. Generated by Claude Code |
Description
The description of
npx skills adddid not match what the installer actually does.It said the tool "writes each one's own convention" and listed
.claude/skills/,.agents/skills/and.windsurf/skills/as peer destinations, with "around seventy others".Running it against a project with Claude Code present shows a different shape. The installer creates one directory per skill under
.agents/skills/— 23 of them,melonjs,melonjs-physics,melonjs-tilemapsand so on — and each detected assistant's directory gets a symlink back to that copy..claude/skills/melonjsis a symlink to../../.agents/skills/melonjs, not a second copy. The tool also prints its own coverage figure, "77 agents", so the passage uses that rather than "around seventy".DOC_README.mdcarried the same passage, so it is corrected in step (raised in review) and the two files no longer contradict each other.Documentation only, no code touched.
Type of change
Checklist
pnpm lintpasses) — the pre-commit hook globs*.{js,ts,cjs,mjs,d.cts,d.mts,jsx,tsx,json,jsonc}, so Markdown is out of scopepnpm testpasses) — verified by running the installer twice and inspecting the resulting tree; no tests cover README prosepnpm build) — unaffected, Markdown onlyRelated issues
None.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Q8aCt5SwH5D5Bi7qKXi2X8