Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions .github/workflows/test-mcp-regression.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 "<missing>"
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: |
Expand Down
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
136 changes: 136 additions & 0 deletions tests/test_skills_symlink_integrity.py
Original file line number Diff line number Diff line change
@@ -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: "<mode> <sha> <stage>\t<path>"
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 "<not a regular file>"
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."
)
Loading