Skip to content

fix: ship skill assets as real copies for Windows compatibility#336

Open
BeyBaba wants to merge 2 commits into
nextlevelbuilder:mainfrom
BeyBaba:fix/windows-symlink-compat
Open

fix: ship skill assets as real copies for Windows compatibility#336
BeyBaba wants to merge 2 commits into
nextlevelbuilder:mainfrom
BeyBaba:fix/windows-symlink-compat

Conversation

@BeyBaba

@BeyBaba BeyBaba commented Jun 6, 2026

Copy link
Copy Markdown

Summary

The skill at .claude/skills/ui-ux-pro-max/ ships data and scripts as filesystem symlinks pointing at ../../../src/ui-ux-pro-max/{data,scripts}. On Windows clones — and any environment where git config core.symlinks defaults to false — git materialises those entries as plain text files containing the symlink target string. The skill then breaks at runtime:

PS> cd .claude/skills/ui-ux-pro-max
PS> python scripts/search.py "fintech crypto" --design-system
# error: scripts/search.py not found / not a directory

This affects users installing the plugin via claude plugin install ui-ux-pro-max@ui-ux-pro-max-skill on Windows.

Fix

  • Replace the two symlinks with real file copies of data/ (1.7 MB, 31 CSVs) and scripts/ (3 Python modules).
  • Add scripts/sync-skill-assets.sh and scripts/sync-skill-assets.ps1 so maintainers can refresh the skill folder after editing the source of truth — mirroring the existing cp -r src/... cli/assets/... workflow already documented for the CLI assets in CLAUDE.md.
  • Update CLAUDE.md Sync Rules to document the new workflow and the Windows rationale.

Net diff: ~9 KB of meaningful changes plus the duplicated CSV/Python content (which was already present once under src/).

Verification

On a fresh Windows clone of this branch:

PS> cd .claude/skills/ui-ux-pro-max
PS> python scripts/search.py "fintech crypto modern" --design-system
# returns the full design system box (pattern, style, colors, typography, ...)

The sync script was also exercised:

bash scripts/sync-skill-assets.sh
# Syncing data/    -> .../skills/ui-ux-pro-max/data
# Syncing scripts/ -> .../skills/ui-ux-pro-max/scripts
# Done. (no git diff after — idempotent)

Trade-off

Duplicating ~1.8 MB of CSV/Python in the repo in exchange for cross-platform install correctness. The duplication is fenced behind a one-line sync command, matching the precedent already set by cli/assets/ in this repo.

Test plan

  • CI Claude Review passes
  • Reviewer runs python scripts/search.py "..." --design-system from inside .claude/skills/ui-ux-pro-max/ on Windows (or with core.symlinks=false) and confirms output
  • Reviewer runs bash scripts/sync-skill-assets.sh (or pwsh scripts/sync-skill-assets.ps1) and confirms no spurious diff

Closes any open Windows-install issues.

BeyBaba added 2 commits June 6, 2026 13:54
… compatibility

The skill at .claude/skills/ui-ux-pro-max/ used filesystem symlinks
(data -> ../../../src/ui-ux-pro-max/data, same for scripts) to avoid
duplicating the source-of-truth content. On Windows clones — and any
environment where `git config core.symlinks` defaults to false — git
materialises these as plain text files containing the symlink target,
which breaks the skill at runtime:

  python scripts/search.py "..." --design-system
  -> "not a directory" / "no such file"

This change replaces the two symlinks with real copies of `data/` and
`scripts/`, and adds a sync script for maintainers to run before commit
whenever the source of truth changes (mirroring the existing
cli/assets/ copy-and-sync pattern documented in CLAUDE.md).

Changes
-------
- Replace .claude/skills/ui-ux-pro-max/{data,scripts} symlinks with file copies
- Add scripts/sync-skill-assets.sh (bash) and scripts/sync-skill-assets.ps1 (pwsh)
- Update CLAUDE.md Sync Rules to document the new workflow

Verification
------------
On Windows the previously broken invocation now works:

  cd .claude/skills/ui-ux-pro-max
  python scripts/search.py "fintech crypto modern" --design-system
Adds .github/workflows/check-asset-sync.yml that fails a PR when the
copy targets diverge from src/ui-ux-pro-max/ (source of truth):

  - .claude/skills/ui-ux-pro-max/{data,scripts}  -> hard-fail (introduced by this PR)
  - cli/assets/{data,scripts,templates}           -> soft-fail (drift today; flip
                                                    after nextlevelbuilder#330 lands)

This addresses the "Optional follow-up" in nextlevelbuilder#330: a CI check so the
documented `cp -r src/... <target>/...` step can't be silently
forgotten on the next source update. Workflow runs on pull_request
and push-to-main when any of these paths change, and prints the exact
sync commands when drift is detected.

The cli/assets/ leg is `continue-on-error: true` for now because
main currently drifts from src/ (the exact bug nextlevelbuilder#330 fixes). Once
that PR merges and main is clean, drop continue-on-error so cli
drift hard-fails too.
@mrgoonie

Copy link
Copy Markdown
Contributor

Summary: I’m deferring full maintainer review for this run because this PR is oversized for the cron-safe review budget and materially overlaps the active packaging/asset-sync queue.

Risk level: Medium

Mandatory gates:

Findings:

  • Important: This PR changes 40 files and adds ~9.4k lines, mostly shipping real copied assets. That is too broad to safely merge without proving which files are canonical source, which files are generated/package artifacts, and what command keeps them in sync.

Verdict: DEFERRED

Next step: please add a short implementation note covering the source-of-truth files, exact sync/copy command, Windows verification path, and how this differs from the other open packaging PRs. If possible, split this into a small workflow/sync fix plus a generated-assets update so review can focus on the actual logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:github-maintain Processed by github-maintain automation maintain:deferred Deferred by maintain workflow pr:oversized Too large for cron-safe review budget

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants