Fix: report symlinked recursive skill skips in completeness (GH-495)

src/skillspector/cli.py

@@ -2710,6 +2710,10 @@ def _scan_multi_skill(
     aggregate_limitations = [
         f"recursive discovery {limitation.resource} limit reached"
         for limitation in detection.limitations[:256]
+        # Symlink omissions get their own clearer aggregate message below;
+        # listing the generic one too would double-report the same entries.
+        if limitation.resource != "multi_skill_symlinked_entry"
+        or not detection.omitted_symlink_entries
     ]
     retained_public_records = 0
     retained_report_characters = 0
@@ -2843,6 +2847,19 @@ def _scan_multi_skill(
         len(skills) - scanned_skill_count,
     )
     output_omitted_skill_count = max(0, scanned_skill_count - len(processed_skills))
+    omitted_symlink_entry_count = detection.omitted_symlink_entries
+    if omitted_symlink_entry_count:
+        analysis_incomplete = True
+        aggregate_limitations.append(
+            f"{omitted_symlink_entry_count} symlinked recursive skill(s) omitted "
+            "(directory symlinks are not followed)"
+        )
+        progress_console.print(
+            f"[yellow]Warning:[/yellow] {omitted_symlink_entry_count} symlinked skill "
+            "directories were skipped during recursive discovery and are not "
+            "included in this scan."
+        )
+    skills_omitted_total = unscanned_skill_count + omitted_symlink_entry_count
     if output_omitted_skill_count:
         analysis_incomplete = True
         aggregate_limitations.append(
@@ -2856,11 +2873,11 @@ def _scan_multi_skill(
         )
     aggregate_limitations = list(dict.fromkeys(aggregate_limitations))[:256]
     aggregate_completeness = _multi_skill_analysis_completeness(
-        total_skills=len(skills),
+        total_skills=len(skills) + omitted_symlink_entry_count,
         complete_skills=complete_skill_count,
         partial_skills=partial_skill_count,
         failed_skills=failed_skill_count,
-        omitted_skills=unscanned_skill_count,
+        omitted_skills=skills_omitted_total,
         limitations=aggregate_limitations,
     )
     analysis_incomplete = not bool(aggregate_completeness["is_complete"])
@@ -2898,10 +2915,15 @@ def _scan_multi_skill(
         progress_console.print(
             f"  {'<unscanned>':<30} {'—':<8} {'—':<12} {unscanned_skill_count:<10} {'partial':<10}"
         )
-    if output_omitted_skill_count or unscanned_skill_count:
+    if omitted_symlink_entry_count:
+        progress_console.print(
+            f"  {'<symlink omitted>':<30} {'—':<8} {'—':<12} "
+            f"{omitted_symlink_entry_count:<10} {'skipped':<10}"
+        )
+    if output_omitted_skill_count or unscanned_skill_count or omitted_symlink_entry_count:
         progress_console.print(
             "[yellow]Recursive scan incomplete:[/yellow] one or more skills were omitted "
-            "after an aggregate safety limit."
+            "after an aggregate safety limit or skipped as symlinks."
         )
 
     if format == FormatChoice.json:
@@ -2914,7 +2936,7 @@ def _scan_multi_skill(
             "risk_recommendation": aggregate_risk_assessment["recommendation"],
             "analysis_completeness": aggregate_completeness,
             "skills_scanned": scanned_skill_count,
-            "skills_omitted": unscanned_skill_count,
+            "skills_omitted": skills_omitted_total,
             "skills_output_omitted": output_omitted_skill_count,
             "public_finding_records": retained_public_records,
             "report_characters": retained_report_characters,
@@ -2965,6 +2987,14 @@ def _scan_multi_skill(
                     "reason": "aggregate_scan_limit",
                 }
             )
+        if omitted_symlink_entry_count:
+            combined_skills.append(
+                {
+                    "omitted": True,
+                    "omitted_count": omitted_symlink_entry_count,
+                    "reason": "symlink_not_followed",
+                }
+            )
         rendered = json.dumps(combined, indent=2)
         if len(rendered) > _MULTI_SKILL_MAX_REPORT_CHARACTERS:
             analysis_incomplete = True

src/skillspector/multi_skill.py

@@ -132,6 +132,7 @@ class MultiSkillDetectionResult:
     entries_examined: int = 0
     structured_candidates_examined: int = 0
     structured_input_bytes_examined: int = 0
+    omitted_symlink_entries: int = 0
 
     @property
     def complete(self) -> bool:
@@ -277,32 +278,32 @@ def detect_skills(directory: Path) -> MultiSkillDetectionResult:
 
         skills: list[SkillDirectory] = []
         limitations: list[MultiSkillDetectionLimitation] = []
+        omitted_symlink_entries = 0
         for entry in _bounded_scandir(directory, budget=budget):
             budget.check_runtime()
             child = Path(entry.path)
+            if entry.name in _SKIP_DIRS:
+                # Intentionally ignored names (e.g. `.git`, `.venv`,
+                # `node_modules`) are not a discovery gap even when they are
+                # symlinks; skip them before recording any limitation. Any
+                # other symlinked name is recorded below, so an eligible
+                # dot-prefixed skill such as `.review-helper` is never
+                # silently excluded.
+                continue
             try:
                 if entry.is_symlink() or _is_link_or_junction(child):
-                    if entry.name in _SKIP_DIRS:
-                        # Intentionally ignored names (e.g. `.git`, `.venv`,
-                        # `node_modules`) are not a discovery gap even when
-                        # they are symlinks; skip them before recording the
-                        # symlink limitation. Any other symlinked name is
-                        # recorded below, so an eligible dot-prefixed skill
-                        # such as `.review-helper` is never silently excluded.
-                        continue
                     limitations.append(
                         MultiSkillDetectionLimitation(
                             reason_code="read_error",
                             resource="multi_skill_symlinked_entry",
                         )
                     )
+                    omitted_symlink_entries += 1
                     continue
                 if not entry.is_dir(follow_symlinks=False):
                     continue
             except OSError as exc:
                 raise _read_error("multi_skill_directory_entry") from exc
-            if entry.name in _SKIP_DIRS:
-                continue
 
             has_manifest = _has_skill_md(child, budget=budget)
             is_structured = False
@@ -333,6 +334,7 @@ def detect_skills(directory: Path) -> MultiSkillDetectionResult:
         entries_examined=budget.entries,
         structured_candidates_examined=budget.structured_candidates,
         structured_input_bytes_examined=budget.structured_bytes,
+        omitted_symlink_entries=omitted_symlink_entries,
     )
 
 

tests/test_multi_skill.py

@@ -341,6 +341,7 @@ def test_dot_prefixed_child_skill_is_discovered_with_explicit_skips(
             "skill-b",
         }
         assert [skill.local_only for skill in result.skills] == [True, False, False]
+        assert result.omitted_symlink_entries == 1
 
     def test_symlinked_skill_directory_marks_discovery_incomplete(self, tmp_path: Path) -> None:
         """Detection must not silently claim complete coverage through a directory symlink."""
@@ -363,6 +364,7 @@ def test_symlinked_skill_directory_marks_discovery_incomplete(self, tmp_path: Pa
         assert result.complete is False
         assert result.limitations[0].reason_code == "read_error"
         assert result.limitations[0].resource == "multi_skill_symlinked_entry"
+        assert result.omitted_symlink_entries == 1
 
     def test_ignored_name_symlinks_do_not_mark_discovery_incomplete(self, tmp_path: Path) -> None:
         """Symlinks with intentionally ignored names are skipped, not recorded.
@@ -445,6 +447,31 @@ def test_eligible_dot_prefixed_symlink_is_not_silently_excluded(self, tmp_path:
         assert result.complete is False
         assert [lim.resource for lim in result.limitations] == ["multi_skill_symlinked_entry"]
         assert result.limitations[0].reason_code == "read_error"
+        assert result.omitted_symlink_entries == 1
+
+    def test_ignored_name_symlink_is_not_counted_as_omitted(self, tmp_path: Path) -> None:
+        """An ignored-name symlink does not inflate the omission count."""
+        for name in ("skill-a", "skill-b"):
+            sub = tmp_path / name
+            sub.mkdir()
+            (sub / "SKILL.md").write_text(f"---\nname: {name}\n---\n", encoding="utf-8")
+        ignored_target = tmp_path.parent / f"{tmp_path.name}-ignored-target"
+        ignored_target.mkdir()
+        (ignored_target / "SKILL.md").write_text("---\nname: mod\n---\n", encoding="utf-8")
+        linked_target = tmp_path.parent / f"{tmp_path.name}-linked-target"
+        linked_target.mkdir()
+        (linked_target / "SKILL.md").write_text("---\nname: linked\n---\n", encoding="utf-8")
+        try:
+            (tmp_path / "node_modules").symlink_to(ignored_target, target_is_directory=True)
+            (tmp_path / "linked-skill").symlink_to(linked_target, target_is_directory=True)
+        except OSError:
+            pytest.skip("symlinks are not supported on this filesystem")
+
+        result = detect_skills(tmp_path)
+
+        assert result.is_multi_skill is True
+        assert {skill.name for skill in result.skills} == {"skill-a", "skill-b"}
+        assert result.omitted_symlink_entries == 1
 
     def test_symlinked_root_is_not_detected(self, tmp_path: Path) -> None:
         """Direct callers cannot use detection to inspect a symlinked root."""

tests/unit/test_cli.py

@@ -1857,6 +1857,131 @@ def test_recursive_markdown_report_character_limit_is_explicit(
     assert len(body) <= 1_024
 
 
+def test_recursive_symlinked_skills_are_reported_as_omitted(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+) -> None:
+    """Symlinked skill directories surface as omitted, not complete coverage."""
+    skill = SkillDirectory(tmp_path / "one", "one", "one")
+    detection = MultiSkillDetectionResult(
+        is_multi_skill=True,
+        skills=[skill],
+        limitations=(
+            MultiSkillDetectionLimitation(
+                reason_code="read_error",
+                resource="multi_skill_symlinked_entry",
+            ),
+        ),
+        omitted_symlink_entries=1,
+    )
+    output = tmp_path / "combined.json"
+    monkeypatch.setattr(
+        cli.graph,
+        "invoke",
+        lambda *_args, **_kwargs: _bounded_recursive_result("one", finding_count=0),
+    )
+
+    _scan_multi_skill(detection, FormatChoice.json, output, no_llm=True)
+
+    payload = json.loads(output.read_text(encoding="utf-8"))
+    assert payload["skills_scanned"] == 1
+    assert payload["skills_omitted"] == 1
+    assert payload["analysis_completeness"]["is_complete"] is False
+    assert payload["analysis_completeness"]["total_files"] == 2
+    assert payload["analysis_completeness"]["coverage_percent"] == 50.0
+    assert payload["analysis_completeness"]["entirely_uninspected_files"] == 1
+    assert payload["risk_recommendation"] == "CAUTION"
+    assert any(
+        "symlinked recursive skill(s) omitted" in limitation
+        for limitation in payload["analysis_completeness"]["limitations"]
+    )
+    assert not any(
+        "multi_skill_symlinked_entry limit reached" in limitation
+        for limitation in payload["analysis_completeness"]["limitations"]
+    )
+    assert payload["skills"][-1] == {
+        "omitted": True,
+        "omitted_count": 1,
+        "reason": "symlink_not_followed",
+    }
+
+
+def _symlink_only_root(tmp_path: Path, *, with_ignored_name: bool) -> Path:
+    """Build a root holding no real skill, only symlinked children."""
+    external = tmp_path / "external"
+    external.mkdir()
+    (external / "SKILL.md").write_text("---\nname: linked\n---\n# benign skill\n", encoding="utf-8")
+    root = tmp_path / "root"
+    root.mkdir()
+    try:
+        (root / "linked-skill").symlink_to(external, target_is_directory=True)
+        if with_ignored_name:
+            (root / "node_modules").symlink_to(external, target_is_directory=True)
+    except OSError:
+        pytest.skip("symlinks are not supported on this filesystem")
+    return root
+
+
+def test_recursive_symlink_only_root_fails_strict_gate(tmp_path: Path) -> None:
+    """Zero real children with one eligible link stays partial end to end.
+
+    The fallback dispatch bypasses `_scan_multi_skill`, so this covers the
+    CLI path the direct aggregate test cannot reach: incomplete JSON and a
+    failing `--fail-on-incomplete` gate with no findings to blame.
+    """
+    root = _symlink_only_root(tmp_path, with_ignored_name=False)
+    output = tmp_path / "report.json"
+
+    result = runner.invoke(
+        app,
+        ["scan", str(root), "--recursive", "--format", "json", "--no-llm", "-o", str(output)],
+    )
+
+    assert result.exit_code == 0
+    payload = json.loads(output.read_text(encoding="utf-8"))
+    assert payload["analysis_completeness"]["is_complete"] is False
+    assert payload["analysis_completeness"]["status"] == "partial"
+
+    strict = runner.invoke(
+        app,
+        [
+            "scan",
+            str(root),
+            "--recursive",
+            "--format",
+            "json",
+            "--no-llm",
+            "-o",
+            str(tmp_path / "strict.json"),
+            "--fail-on-incomplete",
+        ],
+    )
+    assert strict.exit_code == 1
+
+
+def test_recursive_symlink_only_root_keeps_ignored_names_exempt(
+    tmp_path: Path,
+) -> None:
+    """An ignored-name link beside an eligible one adds no discovery gap."""
+    root = _symlink_only_root(tmp_path, with_ignored_name=True)
+    output = tmp_path / "report.json"
+
+    runner.invoke(
+        app,
+        ["scan", str(root), "--recursive", "--format", "json", "--no-llm", "-o", str(output)],
+    )
+
+    assert output.exists()
+    payload = json.loads(output.read_text(encoding="utf-8"))
+    assert payload["analysis_completeness"]["is_complete"] is False
+    discovery = [
+        event
+        for event in payload["analysis_completeness"]["ledger_exceptions"]
+        if event["phase"] == "multi_skill_discovery"
+    ]
+    assert len(discovery) == 1
+    assert discovery[0]["reason_code"] == "read_error"
+
+
 def test_recursive_json_bounds_the_final_serialized_document(
     monkeypatch: pytest.MonkeyPatch, tmp_path: Path
 ) -> None: