[sonichi]

What

check_skill_symlinks prints a repair command when a skill destination is a real directory instead of a symlink. It told you to move the directory aside to <dst>/<name>.local-backup — inside <dst>, which is the directory the skill loader scans.

Why it matters — reproduced on a live core, not theorised

I hit the warning today, ran the emitted command verbatim, and the harness immediately registered the backup as a skill:

⚠ skill-symlinks  1 a real dir, not a link ...: skill-usage-report

$ mv "<dst>/skill-usage-report" "<dst>/skill-usage-report.local-backup" \
  && ln -s "<src>/skill-usage-report" "<dst>/skill-usage-report"

→ The following skills are available for use with the Skill tool:
  - skill-usage-report.local-backup: skill-usage-report      ← phantom duplicate

The symlink repair itself works. The command just leaves a second problem behind, and the operator has no reason to look for it — the probe goes green.

This is easy to miss precisely because the surrounding advice is hard-won: #2660 fixed the quoting so a spaced path no longer silently no-ops, and the comment block explains that at length. The destination was never the part under examination.

Change

-`mv "<dst>/<name>" "<dst>/<name>.local-backup" && ln -s "<src>/<name>" "<dst>/<name>"`
+`mv "<dst>/<name>" "<dst>/../<name>.skill-backup" && ln -s "<src>/<name>" "<dst>/<name>"`

Outside the scanned directory, using only placeholders the message already carries. The quoting property #2660 established is preserved, and its test still enforces it.

Evidence

tests/health-check-dangling-symlinks.test.py already executes the emitted command rather than string-matching it, so it extends naturally. It now asserts the backup lands outside <dst>, plus a new assertion that nothing named *.skill-backup remains inside it.

Both fail against the previous recommendation — verified by restoring the old string rather than assuming:

# old recommendation, new assertions
Ran 14 tests in 0.454s
FAILED (failures=2)
  AssertionError: the remedy did not preserve the local edits it moved aside

# with this change
Ran 14 tests in 0.460s
OK

Scope

Two files, one behaviour. No change to the quoting, the ln -sfn warning, the shadow detection, or any other probe.

Stand: Echo Act IV Pro

🤖 Generated with Claude Code

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 78%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

No lines with coverage information in this diff.

[qingyun-wu]

Reviewed head 30d19a43b22613c65debf339dfc68b684909dee2.

No blocking findings. The remedy now moves a shadowing real skill directory outside the loader-scanned destination before recreating the symlink, so the backup no longer becomes a phantom duplicate skill. The regression test executes the advertised command against a spaced-path fixture and asserts both sides of the behavior: local edits are preserved in <dst>/../<name>.skill-backup, and no backup directory is left inside <dst>.

Checks run in an isolated /private/tmp worktree:

• python3 tests/health-check-dangling-symlinks.test.py
• PYTHONPYCACHEPREFIX=/private/tmp/sutando-pr-2765-30d19a/.pycache python3 -m py_compile src/health-check.py tests/health-check-dangling-symlinks.test.py
• python3 scripts/gen-src-map.py --check
• git diff --check origin/main...HEAD
• git diff origin/main...HEAD | bash scripts/review-checks.sh

Hosted status at review time: diff coverage and fast gates are green; clean-install was still running.

Reviewed by Qingyun's Personal Codex.

[bassilkhilo-ag2]

Verified in an isolated worktree off a freshly-fetched origin/main (dev checkout left untouched):

• python3 tests/health-check-dangling-symlinks.test.py -v at HEAD: 14/14 pass.
• Reverted only src/health-check.py back to origin/main while keeping the PR's updated tests: reproduces exactly the 2 failures the PR body claims (the remedy did not preserve the local edits it moved aside, and the not-quoted assertion) — confirms the new tests actually pin the new behavior rather than passing vacuously.
• Note for future readers: my first pass at this diffed against a stale local origin/main (hadn't fetched since before #2664 landed) and briefly looked like #2765 might be stacked on unmerged #2660 — it isn't; #2660 is merged, git merge-base origin/main pr-2765-review matches the fresh origin/main tip exactly.

Small, single-purpose fix: moves the backup destination outside <dst> so the skill loader can't register it as a phantom duplicate. Diff is exactly what the description says — no changes to quoting, ln -sfn handling, or shadow detection itself. Approving.

[github-actions]

@cla-assistant check