test(issue-42): review R1 - a dangling .claude/skills link fails instead of skipping
Path.exists() follows the link, so a dangling top-level .claude/skills read as "absent" and the test skipped: every skill's load path could be broken with the suite still green (codex R1 on PR #47; the exists() branch predates this PR). The check moves into claude_skills_problems(link, skills_dir, names): absence is "neither exists nor is a symlink"; a symlink must resolve to .agents/skills; a directory must hold each skill resolving to the matching .agents/skills/<skill>. test_claude_skills_layouts runs it on synthetic layouts (absent, single link, per-skill links, a link to the wrong skill, a dangling top-level link, a dangling per-skill link), so it is exercised in a worktree too. Replaying the review's probe (a dangling .claude/skills in the tree) now fails; the main checkout's per-skill layout reports nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
This commit is contained in:
1 parent
e1ed2708d8
commit
150b4937f8
1 file changed
+59
-12
@@ -237,17 +237,64 @@ def test_frontmatter_name_matches_directory(skill):
|
||||
assert m and m.group(1) == skill
|
||||
|
||||
|
||||
def test_claude_skills_link_resolves_to_agents_skills():
|
||||
""".claude/skills is local (git-ignored) and comes in two layouts: one link
|
||||
to .agents/skills, or a directory with one link per skill. Either way each
|
||||
skill Claude Code loads must be the .agents/skills one."""
|
||||
link = REPO / ".claude" / "skills"
|
||||
if not link.exists():
|
||||
pytest.skip(".claude/skills is untracked; absent in a worktree")
|
||||
def claude_skills_problems(link, skills_dir, names):
|
||||
"""None when ``link`` (.claude/skills) is absent; else the list of problems.
|
||||
|
||||
.claude/skills is local (git-ignored) and comes in two layouts: one link to
|
||||
.agents/skills, or a directory with one link per skill. Either way each
|
||||
skill Claude Code loads must be the .agents/skills one. A dangling
|
||||
top-level link is a broken layout, not an absent one: Path.exists()
|
||||
follows the link, so absence is "neither exists nor is a symlink"."""
|
||||
if not link.exists() and not link.is_symlink():
|
||||
return None
|
||||
if link.is_symlink():
|
||||
assert link.resolve() == SKILLS.resolve()
|
||||
return
|
||||
for skill in SKILL_NAMES:
|
||||
ok = link.exists() and link.resolve() == skills_dir.resolve()
|
||||
return [] if ok else [f"{link} -> {link.resolve()} is not {skills_dir}"]
|
||||
problems = []
|
||||
for skill in names:
|
||||
entry = link / skill
|
||||
assert entry.exists(), f".claude/skills/{skill} is missing"
|
||||
assert entry.resolve() == (SKILLS / skill).resolve(), f".claude/skills/{skill} -> {entry.resolve()}"
|
||||
if not entry.exists():
|
||||
problems.append(f"{skill} is missing or dangling")
|
||||
elif entry.resolve() != (skills_dir / skill).resolve():
|
||||
problems.append(f"{skill} -> {entry.resolve()}")
|
||||
return problems
|
||||
|
||||
|
||||
def test_claude_skills_link_resolves_to_agents_skills():
|
||||
problems = claude_skills_problems(REPO / ".claude" / "skills", SKILLS, SKILL_NAMES)
|
||||
if problems is None:
|
||||
pytest.skip(".claude/skills is untracked; absent in a worktree")
|
||||
assert problems == []
|
||||
|
||||
|
||||
@pytest.mark.parametrize("layout,ok", [("absent", None), ("single", True), ("per_skill", True),
|
||||
("wrong_skill", False), ("broken_top", False), ("broken_entry", False)])
|
||||
def test_claude_skills_layouts(tmp_path, layout, ok):
|
||||
"""The check on synthetic layouts, so it is exercised in a worktree too."""
|
||||
skills = tmp_path / ".agents" / "skills"
|
||||
names = ["multi-agent-mux-a", "multi-agent-mux-b"]
|
||||
for n in names:
|
||||
(skills / n).mkdir(parents=True)
|
||||
link = tmp_path / ".claude" / "skills"
|
||||
link.parent.mkdir()
|
||||
if layout == "single":
|
||||
link.symlink_to("../.agents/skills")
|
||||
elif layout == "broken_top":
|
||||
link.symlink_to("../.agents/no-such-dir")
|
||||
elif layout in ("per_skill", "wrong_skill", "broken_entry"):
|
||||
link.mkdir()
|
||||
for n in names:
|
||||
(link / n).symlink_to(f"../../.agents/skills/{n}")
|
||||
if layout == "wrong_skill":
|
||||
(link / names[1]).unlink()
|
||||
(link / names[1]).symlink_to(f"../../.agents/skills/{names[0]}")
|
||||
if layout == "broken_entry":
|
||||
(link / names[1]).unlink()
|
||||
(link / names[1]).symlink_to("../../.agents/skills/gone")
|
||||
problems = claude_skills_problems(link, skills, names)
|
||||
if ok is None:
|
||||
assert problems is None
|
||||
elif ok:
|
||||
assert problems == []
|
||||
else:
|
||||
assert problems, layout
|
||||
Reference in new issue
Block a user