fix(cli): preserve full per-skill JSON payload in recursive scans (#228)

[rodboev]

Summary

skillspector scan --recursive --format json currently reduces each successful skills[] entry to a summary row and drops the full per-skill report contract that single-skill JSON already exposes. This rebases the branch onto current main, preserves the recursive JSON contract for integrations by embedding the full per-skill payload, and keeps current main's combined-file behavior for recursive non-JSON --output.

Closes #228

Root cause

_scan_multi_skill() already has the full graph result for each skill, including the rendered JSON report_body. The recursive JSON writer rebuilds each skills[] entry from only name, path, risk_score, risk_severity, and finding_count, so the richer per-skill JSON document is discarded before the combined file is written.

The branch had also diverged from current main's CLI ownership: cleanup now lives in skillspector.cleanup.cleanup_result, the risk threshold comes from constants.RISK_THRESHOLD, and recursive non-JSON --output uses the combined-file writer already on main.

Diff Notes

• Rebase onto current main, keeping the existing _result_body() helper and concatenated non-JSON --output writer.
• Preserve current-main cleanup and risk-threshold ownership while replaying only the recursive JSON payload behavior.
• Parse successful per-skill JSON report_body values and merge them into each successful recursive skills[] entry while retaining the compatibility summary keys.
• Keep per-skill error entries unchanged.
• Update CLI regressions for recursive JSON fidelity, single-skill preservation, and recursive Markdown --output file behavior.

Scope

This stays on the CLI/report-contract boundary. It does not change analyzers, report-node schema, SARIF output, structured-skill discovery, baseline threading, provider behavior, or non-recursive output.

Verification

• pytest tests/unit/test_cli.py - pass, 12 passed
• ruff check src/ tests/
• ruff format --check src/ tests/
• Invariant enumeration gate - pass, 10 rows, state domains checked
• Local adversarial review - no findings
• CI Lint & Test (Python 3.12), Lint & Test (Python 3.13), and DCO Check - pending maintainer approval and rerun after push

[rng1995]

The JSON payload implementation is sound on this branch's base, but the PR conflicts with current main and its non-JSON regression test locks in behavior that main has since fixed. Rebase, preserve current main's combined-file behavior for recursive non-JSON --output, integrate the JSON payload work, and rerun the CLI suite.

[rng1995]

[Automated SkillSpector Review]

Re-review: still requesting changes. The stale non-JSON regression is corrected and the recursive JSON payload logic looks sound, but GitHub still reports the branch as conflicted with current main. Rebase and preserve main’s centralized cleanup and risk-threshold changes, then rerun the CLI suite.

[rng1995]

[Automated SkillSpector Review]

Re-review: approving. Head bb41e37 replays the branch onto current main and resolves all previously raised blockers. The change embeds the full parsed per-skill JSON report_body into each successful recursive skills[] entry while retaining the compatibility summary keys (name, path, risk_score, risk_severity, finding_count), leaving error entries, single-skill output, SARIF, and non-JSON recursive --output untouched.

Prior-issue resolution checklist

• Stale non-JSON regression test locking in pre-main behavior (2026-06-30 review) — Resolved. New test_cli_scan_recursive_terminal_output_to_file asserts the combined report file is written with --- alpha --- separators and per-skill report bodies, matching main's writer (--- {skill.relative_path} ---), and the existing markdown test now asserts --- skill1 --- / --- skill2 --- in the file.
• Branch unmergeable with main (2026-07-09 review) — Resolved. GitHub now reports the PR as mergeable; the branch is a single commit (bb41e37) whose diff applies cleanly against base c2d09df (current main) — the diff's base blob for src/skillspector/cli.py (e7f8e2db) matches main's blob exactly.
• Preserve main's centralized cleanup (skillspector.cleanup.cleanup_result) and constants.RISK_THRESHOLD (2026-07-09 review) — Resolved. The head diff touches only the recursive-JSON entry construction in _scan_multi_skill plus tests; the rest of cli.py is byte-identical to main, which imports cleanup_result and RISK_THRESHOLD and uses them at the scan exit-threshold and cleanup sites.

New-commit assessment

• _recursive_json_payload() is defensive and correct: only a str report_body that parses to a JSON object is merged; non-JSON, missing, and non-dict (e.g. "[]") payloads fall back to the summary-only entry, so terminal/markdown-formatted recursive runs and partial results cannot corrupt the JSON contract.
• Summary keys are re-asserted after entry.update(payload), so a hostile or malformed report body cannot clobber name/path/risk_score/risk_severity/finding_count. Top-level combined keys (multi_skill, skill_count, max_risk_score) and error-entry shape are unchanged — backward compatible for integrations.
• Tests are thorough: full-payload merge, non-JSON / missing / non-dict report_body, error skill, single-skill JSON contract preservation, and recursive markdown --output file behavior. Exit-code expectations line up with RISK_THRESHOLD = 50 (max score 45 → exit 0).

Non-blocking nits

• In _scan_multi_skill, the five summary values are computed twice: once when building the initial entry dict and again when re-asserting them after entry.update(payload). Computing the summary dict once and reusing it (e.g. summary = {...}; entry = {**summary, **payload}; entry.update(summary)) would remove the duplication, including the double finding_count len(...) computation.
• Minor collateral churn from the replay (two removed explanatory comments and reformatting in existing tests) is cosmetic and acceptable.

[rng1995]

Non-blocking: the five summary values are computed twice — once in the initial entry literal and again here after entry.update(payload). Consider building the summary dict once and reusing it, e.g. summary = {...}; entry = {**summary, **payload}; entry.update(summary), which also avoids recomputing the finding_count len(...) expression.