diff --git a/.github/workflows/test-mcp-regression.yml b/.github/workflows/test-mcp-regression.yml index 561c3e27..ba168c5b 100644 --- a/.github/workflows/test-mcp-regression.yml +++ b/.github/workflows/test-mcp-regression.yml @@ -35,6 +35,69 @@ jobs: - name: Install dependencies run: pip install -e ".[test]" pytest-html + # ── Symlink materialization gate (#357 sub-task 4) ───────────── + # PR #307 replaced .claude/skills/bicameral-* duplicates with symlinks + # to canonical skills/. On Windows, git stores these as mode-120000 + # entries but checks them out as plain text files containing the + # target path string UNLESS core.symlinks=true was set before clone. + # The result is silent breakage — Claude Code's slash-command resolver + # can't follow the broken symlinks. The runtime tests pass because + # they don't go through the slash-command surface, so this needs a + # dedicated gate. + # + # `git ls-files -s` shows mode-120000 for tracked symlinks on every + # platform; that's our cross-platform source of truth. We then check + # actual materialization differs by platform: POSIX should see a + # real symlink (Path.is_symlink()), Windows with core.symlinks=true + # should also see a real symlink, Windows without should see a plain + # file. Failing this catches misconfigured Windows clones AND any + # regression that drops a tracked symlink to a plain file. + - name: Assert .claude/skills/ symlinks are mode-120000 in git + shell: bash + run: | + count=$(git ls-files -s .claude/skills/ | awk '$1 == "120000"' | wc -l | tr -d ' ') + echo "Tracked symlink entries in .claude/skills/: $count" + if [ "$count" -lt 22 ]; then + echo "::error::Expected at least 22 mode-120000 entries under .claude/skills/, got $count." + echo "::error::PR #307 established the symlink contract; a regression here means a" + echo "::error::skill mirror has been re-committed as a plain file (or was deleted)." + exit 1 + fi + - name: Assert symlinks materialize on this platform + shell: bash + # ASCII-only print messages — Windows Python defaults to cp1252 for + # stdout and crashes with UnicodeEncodeError on em-dash / arrow chars. + # PYTHONIOENCODING=utf-8 would also work but ASCII is simpler. + run: | + python - <<'PY' + import os, sys + from pathlib import Path + probe = Path(".claude/skills/bicameral-preflight") + if not probe.exists(): + print(f"::error::{probe} missing entirely") + sys.exit(1) + if probe.is_symlink(): + print(f"OK - {probe} resolves as a symlink -> {os.readlink(probe)}") + sys.exit(0) + # If we land here on Windows, core.symlinks=false swallowed the + # symlink and the file now contains the target path as text. + content = probe.read_text().strip() if probe.is_file() else "" + if content.startswith("../../skills/"): + print( + "::error::Windows clone without core.symlinks=true - " + ".claude/skills/bicameral-preflight is a plain file containing the " + f"path string {content!r} instead of resolving to the canonical " + "skills/ directory. Set `core.symlinks=true` before cloning (or " + "clone via WSL). See CLAUDE.md 'Canonical Skill Source' section." + ) + sys.exit(1) + print( + f"::error::Unexpected state - {probe} is neither a symlink nor a " + f"path-string text file: {content!r}" + ) + sys.exit(1) + PY + # ── Build code locator index ─────────────────────────────────── - name: Build code locator index run: | diff --git a/CLAUDE.md b/CLAUDE.md index 69fb3a51..5c84033c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,7 +4,7 @@ `skills/` is the **single canonical location** for all skill files in this project. `.claude/skills/bicameral-*` are symlinks to `../../skills/bicameral-*` — they exist so Claude Code's slash-command resolver finds the skills, but they always resolve to the canonical content. Edit only the `skills/` versions; never write through the symlinks. -> **Windows contributors**: git stores symlinks as mode-120000 entries. Windows defaults to `core.symlinks=false` and stores the symlink *target string* as a plain text file. Set `core.symlinks=true` before cloning (or use WSL) so the symlinks materialize correctly. +> **Windows contributors (required)**: git stores symlinks as mode-120000 entries. Windows defaults to `core.symlinks=false` and stores the symlink *target string* as a plain text file — this breaks slash-command resolution silently. Before cloning, run `git config --global core.symlinks true` (or develop via WSL). If you've already cloned with the default, fix in-place: `git rm --cached .claude/skills/bicameral-* && git checkout -- .claude/skills/`. CI enforces this contract via `tests/test_skills_symlink_integrity.py` and a dedicated step in `test-mcp-regression.yml` (#357 sub-task 4); a Windows clone without `core.symlinks=true` will fail both gates. ## Tool Changes Require Skill Changes (Mandatory) diff --git a/tests/test_skills_symlink_integrity.py b/tests/test_skills_symlink_integrity.py new file mode 100644 index 00000000..b39e32bb --- /dev/null +++ b/tests/test_skills_symlink_integrity.py @@ -0,0 +1,136 @@ +"""Symlink integrity check for `.claude/skills/bicameral-*` (#357 sub-task 4). + +PR #307 replaced the `.claude/skills/bicameral-*` duplicates with symlinks to +the canonical `skills/bicameral-*` source. Git stores those as mode-120000 +entries on every platform, but Windows defaults to `core.symlinks=false` and +materializes them as plain text files containing the target path string +("../../skills/bicameral-preflight") instead of resolving them as symlinks. + +That breakage is silent — the regular MCP test surface still passes because +nothing tries to follow the symlinks from inside the test harness. The +breakage only surfaces when Claude Code's slash-command resolver tries to +resolve `/bicameral-preflight` and finds a plain file where it expected a +symlink to a directory. This test is the early-warning gate. + +CI also has a workflow-level check +(`.github/workflows/test-mcp-regression.yml` — "Assert .claude/skills/ +symlinks…") that fires on every PR. This test duplicates the contract at +the pytest layer so contributors running `pytest tests/` locally see the +same failure with the same actionable error message before they push. + +Setup wizard (`setup_wizard.py::_install_skills`) is NOT the right place +for this check — that path is for wheel installs where the bundled +`skills/` content is *copied* into the target repo's `.claude/skills/`. +There's no symlink at that layer; nothing to check. +""" + +from __future__ import annotations + +import os +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parent.parent +SKILLS_DIR = REPO_ROOT / ".claude" / "skills" + +# PR #307 established 22 symlinks. Lower bound — adding more bicameral-* +# skills will push this up, removing one would be intentional and require +# updating this constant. +EXPECTED_MIN_SYMLINKS = 22 + + +def _git_ls_files() -> list[tuple[str, str]]: + """Return [(mode, path)] for tracked entries under .claude/skills/.""" + result = subprocess.run( + ["git", "ls-files", "-s", ".claude/skills/"], + cwd=REPO_ROOT, + capture_output=True, + text=True, + check=False, + ) + if result.returncode != 0: + pytest.skip( + f"git ls-files failed (not a git repo, or git unavailable): {result.stderr.strip()}" + ) + entries: list[tuple[str, str]] = [] + for line in result.stdout.splitlines(): + # Format: " \t" + parts = line.split("\t", 1) + if len(parts) != 2: + continue + meta = parts[0].split() + if len(meta) < 1: + continue + entries.append((meta[0], parts[1])) + return entries + + +def test_skills_symlinks_tracked_as_mode_120000(): + """The git index must carry .claude/skills/bicameral-* as symlinks. + + Catches a regression where one of those symlinks was accidentally + re-committed as a plain file — the change would compile and ship, + but slash-command resolution would silently drift back to the old + duplicate-and-skew problem PR #307 closed. + """ + entries = _git_ls_files() + symlink_entries = [(m, p) for (m, p) in entries if m == "120000"] + assert len(symlink_entries) >= EXPECTED_MIN_SYMLINKS, ( + f"Expected at least {EXPECTED_MIN_SYMLINKS} mode-120000 symlink entries " + f"under .claude/skills/, got {len(symlink_entries)}.\n" + f"PR #307 (chore(skills)) established the symlink contract. A regression " + f"here means a skill mirror has been re-committed as a plain file (or was " + f"deleted). Inspect with:\n" + f" git ls-files -s .claude/skills/ | awk '$1 != \"120000\"'\n" + f"and either restore the symlink or update {__file__}::" + f"EXPECTED_MIN_SYMLINKS if the change is intentional." + ) + + +def test_skills_symlinks_materialize_on_this_clone(): + """The on-disk checkout must resolve the symlinks as real symlinks. + + Catches Windows clones where `core.symlinks=false` left the entries as + plain text files containing the target path. The failure message + surfaces the specific fix command — contributors should never have to + grep for "how do I fix this" when CI tells them outright. + """ + probe = SKILLS_DIR / "bicameral-preflight" + assert probe.exists(), ( + f"{probe} is missing entirely. Either the .claude/skills/ scaffold " + f"was wiped from the working tree, or `git clone` failed mid-way. " + f"Re-clone or run `git checkout .claude/skills/`." + ) + if probe.is_symlink(): + target = os.readlink(probe) + assert target.startswith("../../skills/"), ( + f"{probe} resolves as a symlink but points to {target!r} — " + f"expected something starting with '../../skills/'. Either the " + f"symlink was edited or the canonical path moved." + ) + return + + # Not a symlink: likely Windows with core.symlinks=false. + content = probe.read_text().strip() if probe.is_file() else "" + if content.startswith("../../skills/"): + pytest.fail( + f"{probe} is a plain file containing the path string {content!r} " + f"instead of a real symlink.\n\n" + f"This is the Windows `core.symlinks=false` failure mode. PR #307 " + f"established the canonical-skill-source contract via symlinks; " + f"Windows clones must opt in to symlink materialization.\n\n" + f"Fix:\n" + f" 1. Set core.symlinks=true GLOBALLY (or per-clone before cloning):\n" + f" git config --global core.symlinks true\n" + f" 2. Re-clone the repo, OR run inside the existing clone:\n" + f" git rm --cached .claude/skills/bicameral-*\n" + f" git checkout -- .claude/skills/\n" + f" 3. Alternative: develop inside WSL where symlinks work natively.\n\n" + f"See CLAUDE.md §'Canonical Skill Source' for the design rationale." + ) + pytest.fail( + f"{probe} is neither a symlink nor a path-string text file. " + f"Unexpected state: {content!r}. Investigate the clone manually." + )