From 150b4937f816c6407be16fc4c37ab88e478927e7 Mon Sep 17 00:00:00 2001
From: Godopu <godopu16@gmail.com>
Date: Sun, 4 Oct 2026 13:46:41 +0900
Subject: [PATCH] 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
---
 tests/test_issue42_skill_blocks.py | 71 +++++++++++++++++++++++++-----
 1 file changed, 59 insertions(+), 12 deletions(-)

diff --git a/tests/test_issue42_skill_blocks.py b/tests/test_issue42_skill_blocks.py
index e7d38de..2d57727 100644
--- a/tests/test_issue42_skill_blocks.py
+++ b/tests/test_issue42_skill_blocks.py
@@ -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