infra(symlinks): #357 sub-task 4 — Windows symlink materialization gate#364
Merged
Conversation
PR #307 replaced .claude/skills/bicameral-* duplicates with symlinks to canonical skills/bicameral-*. 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 — the symlink-as-string failure mode that breaks slash-command resolution silently. PR #307 noted this in passing but shipped no enforcement. This commit closes the loop. Three layers of enforcement (all defense-in-depth): 1. `tests/test_skills_symlink_integrity.py` (NEW) — pytest-level gate. Two assertions: - test_skills_symlinks_tracked_as_mode_120000: git ls-files shows at least 22 mode-120000 entries under .claude/skills/. Catches regressions where a symlink got re-committed as a plain file. - test_skills_symlinks_materialize_on_this_clone: the on-disk .claude/skills/bicameral-preflight resolves as a symlink (or, on Windows with core.symlinks=false, contains the path string '../../skills/...' which triggers a loud failure with the specific fix commands). Runs on every `pytest tests/` invocation in this repo — Windows contributors see the failure with the fix command before they push. 2. `.github/workflows/test-mcp-regression.yml` — CI gate at the workflow level. Two new steps run BEFORE the regular test suite: - Asserts the mode-120000 entry count in git ls-files - Asserts the on-disk materialization via a Python probe Runs on both ubuntu-latest and windows-latest via the existing matrix.os. The windows-latest run is where this actually matters — ubuntu always materializes symlinks correctly. 3. `CLAUDE.md` — Windows note upgraded from "recommended" to "required", with the specific fix-in-place commands (`git rm --cached` + `git checkout`) for contributors who already cloned without core.symlinks set. References the new test + workflow gate so the contract is self-documenting. Why NOT in setup_wizard.py: the wizard path is for *wheel installs*, where bundled skills/ content gets COPIED into the target repo's .claude/skills/. There are no symlinks at that layer — nothing to check. The Windows symlink issue only affects developer clones of this repo itself, where the test + CI gates are the right surface. Verified locally: 2/2 tests pass against the existing macOS clone. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The sub-task 4 workflow step's inline Python script printed em-dash and right-arrow characters. Windows Python defaults to cp1252 for stdout, which can't encode either — UnicodeEncodeError crashed the step right after a successful symlink check. The check itself worked; the diagnostic print is what failed. Replaces em-dashes with hyphens and arrows with '->'. Could also have set PYTHONIOENCODING=utf-8 but ASCII keeps the change scope narrower. Failure log on PR #364: UnicodeEncodeError: 'charmap' codec can't encode character '→' in position 62: character maps to <undefined> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements #357 sub-task 4 (the last sub-task on this tracking issue). PR #307 replaced
.claude/skills/bicameral-*duplicates with symlinks but shipped no enforcement that Windows clones materialize them correctly. This PR closes that loop.Stacks logically alongside the rest of the #357 cluster (#359 merged, #360, #361) but branches independently from
dev— can merge in any order.Why
Git stores the 22
.claude/skills/bicameral-*entries as mode-120000 symlinks. Windows defaults tocore.symlinks=falseand silently materializes them as plain text files containing the target path string ("../../skills/bicameral-preflight"). The MCP test surface still passes because nothing tries to follow the symlinks; the breakage surfaces only when Claude Code's slash-command resolver tries to resolve/bicameral-preflight. PR #307's body noted this in passing but added no gate, so it remained the "verified in-vivo" claim Devin's #357 critique correctly flagged.Three defense-in-depth layers
1.
tests/test_skills_symlink_integrity.py(NEW)Two assertions, both with actionable failure messages:
test_skills_symlinks_tracked_as_mode_120000—git ls-files -sshows ≥ 22 mode-120000 entries under.claude/skills/. Catches the regression where a symlink got re-committed as a plain file (whether by Windows-clone misconfiguration or by a misguided IDE auto-fix).test_skills_symlinks_materialize_on_this_clone— on-disk.claude/skills/bicameral-preflightmust resolve as a real symlink. If it's a plain file containing"../../skills/...", surfaces the specific fix commands inline:Contributors never have to grep for "how do I fix this" when CI tells them outright.
2.
.github/workflows/test-mcp-regression.ymlTwo new steps run BEFORE the regular test suite on both
ubuntu-latestandwindows-latest:git ls-files::error::annotationsThe
windows-latestmatrix slot is where this actually matters —ubuntu-latestalways materializes symlinks correctly. Now the matrix isn't just running tests on Windows; it's gating the symlink contract there too.3.
CLAUDE.mdUpgraded the Windows note from "recommended" framing to "required", with the specific fix-in-place commands AND a pointer to the test + workflow gate that enforces the contract.
Why NOT in
setup_wizard.pyThe wizard's
_install_skillspath is for wheel installs — bundledskills/content is copied into the target repo's.claude/skills/. There are no symlinks at that layer; nothing to check. The Windows symlink issue only affects developer clones of this repo itself, where the test + CI gates are the right surface.Verification
Negative-case verification (manually edited test to assert count > 1000): fails with the actionable error message listing the fix command.
Test plan
ubuntu-latest(the new steps pass cleanly)windows-latest— the more interesting check, since this is where the gate actually trips on misconfigured clonescore.symlinks=falseon a Windows VM, run pytest, confirm the gate fires with the documented fixWhat's left on #357
With this PR, all four acceptance criteria of #357 have a delivered PR:
The remaining work on #357 itself is the 8 trap-row backfill PRs that the sub-task 1 audit identified — sequenced after sub-tasks 2/3/4 land.
🤖 Generated with Claude Code