Skip to content

chore: migrate .claude/skills to directory symlinks#13486

Merged
DeJeune merged 1 commit intomainfrom
chore/skills-symlink
Mar 16, 2026
Merged

chore: migrate .claude/skills to directory symlinks#13486
DeJeune merged 1 commit intomainfrom
chore/skills-symlink

Conversation

@EurFelux
Copy link
Copy Markdown
Collaborator

What this PR does

Before this PR:

.claude/skills/<name>/SKILL.md files were copied from .agents/skills/<name>/SKILL.md via pnpm skills:sync. A dedicated skills-check-windows CI job ran on windows-latest to verify cross-platform file-copy compatibility.

After this PR:

.claude/skills/<name> entries are directory symlinks pointing to ../../.agents/skills/<name>, following the Single Source of Truth (SSoT) principle. The Windows-specific CI job is removed; Windows developers are expected to enable symlink support.

Why we need it and why it was done in this way

The following tradeoffs were made:

  • Windows developers must now manually enable symlink support (Developer Mode + git config --global core.symlinks true). This is acceptable because:
    1. The existing AGENTS.md is already a symlink, so Windows compatibility was never fully enforced.
    2. Symlinks eliminate the need for file-copy synchronization, reducing maintenance complexity.
    3. Contributors are expected to have sufficient technical capability to configure their environments.

The following alternatives were considered:

  • Keeping file-copy sync: rejected because it duplicates content and requires extra CI to verify consistency.

Breaking changes

Windows developers who clone without symlink support enabled will get plain text files instead of symlinks. They must:

  1. Enable Developer Mode or grant SeCreateSymbolicLinkPrivilege
  2. Run git config --global core.symlinks true
  3. Re-clone or run pnpm skills:sync

Special notes for your reviewer

  • The .github/workflows/ci.yml diff includes minor quote-style changes ('") from the YAML formatter — these are cosmetic only.

Checklist

Release note

NONE

Replace file-copy sync with directory symlinks for .claude/skills,
following the Single Source of Truth principle. Update skills:sync
and skills:check scripts, remove skills-check-windows CI job,
and add Windows symlink setup docs.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Signed-off-by: icarus <[email protected]>
Copy link
Copy Markdown
Collaborator

@GeorgeDong32 GeorgeDong32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review completed. The PR correctly migrates .claude/skills from file-copy to symlinks following the Single Source of Truth principle. Tested on Windows with proper symlink configuration - scripts work correctly. Documentation is comprehensive. No issues found.

@DeJeune DeJeune merged commit fe0678a into main Mar 16, 2026
10 checks passed
@DeJeune DeJeune deleted the chore/skills-symlink branch March 16, 2026 00:46
DeJeune added a commit that referenced this pull request Mar 16, 2026
### What this PR does

Before this PR:
No React/Next.js performance best practices skill available for AI
coding agents.

After this PR:
Adds the `vercel-react-best-practices` skill from
`vercel-labs/agent-skills`, providing 60+ React and Next.js performance
optimization rules covering rendering, re-renders, bundling, async
patterns, server components, and JS performance.

### Why we need it and why it was done in this way

The following tradeoffs were made:
- This skill includes ~8 Next.js server-specific rules (`server-*`,
`async-api-routes`) that are not applicable to our Electron + React
project. We chose to keep them rather than fork/trim the skill, since:
(1) AI agents will naturally ignore irrelevant rules based on context,
(2) keeping the full skill makes future upstream updates easier, and (3)
the ~50 React/JS/rendering/bundle rules provide significant value for
our codebase.

The following alternatives were considered:
- Forking the skill and removing Next.js-specific rules — rejected to
avoid maintenance burden and divergence from upstream.
- Writing a custom React-only skill — rejected as the Vercel skill
already covers the React patterns comprehensively.

Links to places where the discussion took place:
- Source: https://github.com/vercel-labs/agent-skills

### Breaking changes

None

### Special notes for your reviewer

- The skill is installed in
`.agents/skills/vercel-react-best-practices/` as the single source of
truth.
- `.claude/skills/vercel-react-best-practices` is a **directory
symlink** pointing to `.agents/skills/vercel-react-best-practices/`,
following the convention established in #13486. This replaces the
earlier approach of copying SKILL.md into a real directory.
- `public-skills.txt` was updated and `pnpm skills:sync` was run to
update `.gitignore` whitelists.
- Windows compatibility: Windows developers need symlink support enabled
(via `git config core.symlinks true` or Developer Mode). This is
consistent with the project-wide symlink convention adopted in #13486,
and confirmed working by @GeorgeDong32 on Windows 11 — Claude Code CLI
can load skills even when symlinks fall back to plaintext path files.
- The skill contains 60+ markdown rule files covering categories:
rendering, re-renders, bundle optimization, async patterns, server
components, client patterns, and JS performance.
- Rule applicability breakdown for our Electron + React project:

  | Category | Count | Applicable |
  |---|---|---|
  | `js-*` (JS perf) | 13 | Yes |
  | `rerender-*` (React) | 12 | Yes |
  | `rendering-*` | 11 | Mostly |
  | `bundle-*` | 5 | Partially |
  | `client-*` | 4 | Yes |
  | `advanced-*` | 3 | Yes |
  | `async-*` | 4 | Partially |
  | `server-*` (Next.js) | 8 | No |

### Checklist

- [x] PR: The PR description is expressive enough and will help future
contributors
- [x] Code: [Write code that humans can
understand](https://en.wikiquote.org/wiki/Martin_Fowler#code-for-humans)
and [Keep it simple](https://en.wikipedia.org/wiki/KISS_principle)
- [x] Refactor: You have [left the code cleaner than you found it (Boy
Scout
Rule)](https://learning.oreilly.com/library/view/97-things-every/9780596809515/ch08.html)
- [x] Upgrade: Impact of this change on upgrade flows was considered and
addressed if required
- [ ] Documentation: A [user-guide update](https://docs.cherry-ai.com)
was considered and is present (link) or not required. Check this only
when the PR introduces or changes a user-facing feature or behavior.
- [x] Self-review: I have reviewed my own code (e.g., via
[`/gh-pr-review`](/.claude/skills/gh-pr-review/SKILL.md), `gh pr diff`,
or GitHub UI) before requesting review from others

### Release note

```release-note
NONE
```

---------

Signed-off-by: suyao <[email protected]>
Signed-off-by: icarus <[email protected]>
Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: icarus <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants