feat(cli): add --recursive flag for multi-skill directory scanning

[mimran-khan]

Summary

When a directory contains multiple independent skills (each with its own SKILL.md) but no root-level SKILL.md, SkillSpector previously treated the entire tree as one monolithic skill — summing all findings into a single inflated score and reporting the skill name as "unknown".

Changes

• multi_skill.py: New module with detect_skills() that identifies directories containing 2+ sub-skills by scanning for SKILL.md in immediate subdirectories. Extracts skill names from frontmatter with directory-name fallback.
• --recursive (-r) CLI flag: When set, invokes the LangGraph workflow once per detected sub-skill, producing per-skill reports and a combined summary table.
• Auto-detection warning: When --recursive is NOT specified but multiple skills are detected, prints a warning guiding users to the correct flag.
• JSON combined output: With --recursive --format json --output, writes a structured combined report with per-skill scores.

Example

$ skillspector scan ./skill-collection/ --recursive

Multi-skill directory detected: 3 skills found

  [1/3] Scanning weather-lookup (weather-lookup/)
         Score: 15/100 (LOW)

  [2/3] Scanning email-sender (email-sender/)
         Score: 42/100 (MEDIUM)

  [3/3] Scanning file-manager (file-manager/)
         Score: 8/100 (LOW)

═══ Multi-Skill Summary ═══

  Skill                          Score    Severity     Findings
  ────────────────────────────── ──────── ──────────── ──────────
  weather-lookup                 15       LOW          3
  email-sender                   42       MEDIUM       7
  file-manager                   8        LOW          2

Design Decisions

Decision	Rationale
Require >= 2 sub-skills for multi-skill detection	Avoids false positives on single-skill repos
Root SKILL.md always wins	Preserves backward compatibility for monorepos with a parent skill
Hidden directories skipped	Standard convention (.git, .venv, etc.)
Graph invoked per sub-skill (not batched)	Each skill gets independent context, manifest, and score

Test Plan

• 12 unit tests covering detection, name extraction, edge cases (empty dirs, single sub-skill, hidden dirs, root override, fallback names, lowercase skill.md)
• Full test suite: 688 passed, 0 failures, 0 regressions
• ruff check and ruff format clean

Fixes #136

[mimran-khan]

PS: These issues were found while running bunch of skill evals using SkillSpector and hence raising all the issues so that i can use these with correct reference to Upstream code

[rng1995]

Review: add --recursive flag for multi-skill directory scanning

Nice feature. The detection logic is appropriately bounded and the detect_skills unit tests are thorough. Approving with non-blocking nits.

Security / robustness (looks good)

• Detection only inspects immediate child directories (one level), skips hidden dirs, and uses yaml.safe_load. The frontmatter regex \n---\s*\n is linear (no ReDoS), and there's no unbounded filesystem walk — so no path-explosion concern.
• One thing to be aware of: child.is_dir() follows symlinks, so a symlinked subdirectory would be scanned. Acceptable for a user-initiated local scan, but worth a comment or an is_symlink() guard if you want to be strict.

Non-blocking nits

1. --recursive is a slight misnomer — it scans only immediate child directories, not a recursive tree. Consider clarifying the help text to avoid surprise.
2. Dead code: the finally: pass inside the per-skill loop can be removed.
3. Per-skill cleanup: the multi-skill path returns before the outer try/finally, so per-skill results never pass through _cleanup_result. Local sub-skill dirs don't create temp dirs today, but if a sub-skill scan ever sets temp_dir_for_cleanup it would leak. Consider cleaning up each result.
4. Typing: _scan_multi_skill(detection: object, ...) + assert isinstance(...) is awkward. Since multi_skill is already imported, import MultiSkillDetectionResult and type the parameter directly.
5. Summary count source: the summary uses result.get("findings") (the raw accumulated list), while score/severity come from filtered_findings. This count can therefore differ from the per-skill report (and from #142's deduped list). Consider using filtered_findings for consistency.
6. Test gap: detect_skills is well covered, but _scan_multi_skill orchestration (the max_score > 50 exit code, combined JSON output) has no tests. Worth adding.

None of these are correctness or security issues, hence the approval.

[mimran-khan]

Thanks for the approval! Addressed the non-blocking nits:

1. Help text clarified — --recursive now says "scan directories containing multiple skills (immediate subdirectories with SKILL.md) independently" instead of just "multi-skill directories"
2. Dead code removed — finally: pass eliminated
3. Typing fixed — _scan_multi_skill now takes MultiSkillDetectionResult directly (imported at top level) instead of object + assert
4. Findings count source — summary table and JSON output now use filtered_findings (falling back to findings) for consistency with per-skill reports

Items 5 (symlink guard) and 6 (orchestration tests) noted for follow-up — keeping this PR focused on the current nits.

[rng1995]

Re-review: --recursive flag (refreshing approval on latest commit)

Re-confirming approval on the current head (484af228) — the follow-up commits addressed the actionable nits from my earlier review:

• --recursive help text clarified to state it scans immediate subdirectories containing SKILL.md.
• Dead finally: pass removed.
• _scan_multi_skill now takes MultiSkillDetectionResult directly (no more object + assert isinstance).
• Summary/JSON counts now use filtered_findings (falling back to findings) for consistency with per-skill reports.

The two deferred items — a symlink is_symlink() guard on child directories, and orchestration tests for _scan_multi_skill's exit-code / combined-JSON paths — are fine as follow-ups; neither is a correctness or security concern. Thanks!

[rng1995]

Re-approval on the latest commit (484af228)

Re-confirming my approval on the current head — the author addressed four of my six non-blocking nits and the remaining items are reasonable follow-ups. Still approving.

Addressed:

1. Help text clarified — --recursive now reads "Scan directories containing multiple skills (immediate subdirectories with SKILL.md) independently," which removes the "recursive tree" surprise.
2. Dead code removed — the finally: pass in the per-skill loop is gone.
3. Typing fixed — _scan_multi_skill(detection: MultiSkillDetectionResult, ...) is now typed directly (imported at the top of cli.py); no more object + assert isinstance.
4. Summary count source — the summary table now uses result.get("filtered_findings") or result.get("findings"), consistent with the per-skill report (and with #142's deduped scoring).

Deferred (acceptable as follow-ups — none are correctness/security issues):

• Optional is_symlink() guard on child directories — fine to skip for a user-initiated local scan.
• Orchestration tests for _scan_multi_skill (the max_score > 50 exit code, combined JSON output) — detect_skills remains well covered; adding these later is reasonable.

(For a future pass: the per-skill results in _scan_multi_skill still return before the outer try/finally, so they don't pass through _cleanup_result — harmless today since sub-skill scans don't create temp dirs, just noting it alongside the other follow-ups.)

Nice feature — approving on 484af228.