From 8eeac8f8e4688d749e615bc899e9bbd662823b09 Mon Sep 17 00:00:00 2001 From: Whj9283 <1621370123@qq.com> Date: Fri, 16 Oct 2026 16:15:11 +0800 Subject: [PATCH 1/2] feat(cli): configure the recursive skill count limit Signed-off-by: Whj9283 <1621370123@qq.com> --- README.md | 17 +++++++ src/skillspector/cli.py | 34 +++++++++++-- tests/unit/test_cli.py | 109 +++++++++++++++++++++++++++++++++++++++- 3 files changed, 155 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index cf3eb6efa..fae99b5c1 100644 --- a/README.md +++ b/README.md @@ -169,6 +169,23 @@ skillspector scan ./my-skill/ --format markdown --output report.md skillspector scan ./my-skill/ --format sarif --output report.sarif ``` +### Recursive Collection Scanning + +`--recursive` scans immediate subdirectories containing `SKILL.md` as independent +skills, with a default aggregate limit of **32 skills**. To raise or lower it: + +```bash +skillspector scan ./my-skills/ --recursive --max-skills 100 --no-llm --format json +``` + +`--max-skills` requires a local recursive collection and a positive integer; `0` +does not mean unlimited. Discovery, byte, finding-record, time and report-size +budgets still apply, so increasing the count does not guarantee complete coverage. +When the count limit is reached, the CLI names the limit and suggests adjusting it +or scanning sub-skills separately. Omitted skills remain visible in the aggregate +report and coverage stays incomplete. Use `--fail-on-incomplete` in CI to fail on +incomplete coverage; the default exit policy is unchanged. + ### Batch Scanning Scan entire directories of skills in parallel from `contrib/batch_scan/`: diff --git a/src/skillspector/cli.py b/src/skillspector/cli.py index 6004a2715..7dd941c31 100644 --- a/src/skillspector/cli.py +++ b/src/skillspector/cli.py @@ -525,6 +525,15 @@ def scan( help="Scan immediate subdirectories that each contain a SKILL.md as independent skills.", ), ] = False, + max_skills: Annotated[ + int | None, + typer.Option( + "--max-skills", + min=1, + help="Maximum skills in a local --recursive multi-skill scan (default: 32). " + "Other aggregate safety limits still apply; no unlimited value.", + ), + ] = None, baseline: Annotated[ Path | None, typer.Option( @@ -674,6 +683,11 @@ def scan( authentication session. codex_cli is registered but disabled because its read-only sandbox permits host-file reads; use another provider. """ + if max_skills is not None and (not recursive or mcp_registry or not Path(input_path).is_dir()): + err_console.print( + "[red]Error:[/red] --max-skills requires a local --recursive multi-skill directory" + ) + raise typer.Exit(code=2) if mcp_registry_compare is not None and not mcp_registry: err_console.print("[red]Error:[/red] --mcp-registry-compare requires --mcp-registry") raise typer.Exit(code=2) @@ -795,6 +809,7 @@ def scan( verbose=verbose, fail_on_incomplete=fail_on_incomplete, fail_on_findings=fail_on_findings, + max_skills=max_skills, min_coverage=min_coverage, ) except typer.Exit: @@ -809,6 +824,12 @@ def scan( err_console.print(f"[red]Error:[/red] {e}") raise typer.Exit(code=2) from e return + if max_skills is not None: + err_console.print( + "[red]Error:[/red] --max-skills requires detected sub-skills; " + "scan this input without --max-skills" + ) + raise typer.Exit(code=2) if detection.complete and not detection.has_root_skill and len(detection.skills) == 0: discovery_console.print( "[yellow]Warning:[/yellow] --recursive specified but no sub-skills " @@ -2920,9 +2941,13 @@ def _scan_multi_skill( fail_on_incomplete: bool = False, min_coverage: float | None = None, fail_on_findings: bool = False, + max_skills: int | None = None, **legacy_kwargs: object, ) -> None: """Scan each detected sub-skill independently and produce a combined report.""" + skill_limit = _MULTI_SKILL_MAX_SKILLS if max_skills is None else max_skills + if isinstance(skill_limit, bool) or not isinstance(skill_limit, int) or skill_limit < 1: + raise ValueError("max_skills must be a positive integer") if yara_dir is None and isinstance(legacy_kwargs.get("yara_rules_dir"), Path): yara_dir = str(legacy_kwargs["yara_rules_dir"]) skills = detection.skills @@ -2964,10 +2989,13 @@ def _scan_multi_skill( coverage_failed = False for i, skill in enumerate(skills, 1): - if i > _MULTI_SKILL_MAX_SKILLS: + if i > skill_limit: analysis_incomplete = True - aggregate_limitations.append( - f"recursive skill count budget {_MULTI_SKILL_MAX_SKILLS} reached" + aggregate_limitations.append(f"recursive skill count budget {skill_limit} reached") + progress_console.print( + f"[yellow]Recursive skill count budget {skill_limit} reached.[/yellow] " + "Use --recursive --max-skills N with a larger positive N, or scan " + "sub-skills separately. Other aggregate safety limits still apply." ) break if retained_public_records >= _MULTI_SKILL_MAX_PUBLIC_RECORDS: diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 6e09b15f2..6f84dc450 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -2432,8 +2432,113 @@ def test_recursive_fail_on_findings_exits_one_below_risk_threshold( assert exit_info.value.exit_code == 1 +@pytest.mark.parametrize(("limit", "scanned"), [(None, 32), (1, 1), (33, 33), (40, 33)]) +@pytest.mark.parametrize("strict", [False, True]) +def test_recursive_configurable_skill_count_budget( + monkeypatch, tmp_path, capsys, limit, scanned, strict +): + skills = [SkillDirectory(tmp_path / str(i), str(i), str(i)) for i in range(33)] + detection = MultiSkillDetectionResult(is_multi_skill=True, skills=skills) + calls = [] + + def fake_scan(**kwargs): + calls.append(kwargs) + return _bounded_recursive_result("safe", finding_count=0) + + monkeypatch.setattr(cli, "_scan_skill", fake_scan) + output = tmp_path / "report.json" + context = pytest.raises(typer.Exit) if strict and scanned < 33 else nullcontext() + with context as error: + _scan_multi_skill( + detection, + FormatChoice.json, + output, + no_llm=True, + max_skills=limit, + fail_on_incomplete=strict, + ) + if strict and scanned < 33: + assert error.value.exit_code == 1 + payload = json.loads(output.read_text()) + assert len(calls) == payload["skills_scanned"] == scanned + assert payload["skills_omitted"] == 33 - scanned + assert payload["analysis_completeness"]["is_complete"] is (scanned == 33) + assert payload["risk_recommendation"] == ("SAFE" if scanned == 33 else "CAUTION") + if scanned < 33: + effective = 32 if limit is None else limit + assert ( + f"recursive skill count budget {effective} reached" + in payload["analysis_completeness"]["limitations"] + ) + assert "--max-skills" in capsys.readouterr().out + + +@pytest.mark.parametrize("value", ["0", "-1", "abc"]) +def test_cli_max_skills_rejects_invalid_count(tmp_path, monkeypatch, value): + monkeypatch.setattr(cli, "_scan_skill", lambda **_: pytest.fail("must not scan")) + result = runner.invoke(app, ["scan", str(tmp_path), "--recursive", "--max-skills", value]) + assert result.exit_code == 2 + + +@pytest.mark.parametrize("recursive", [False, True]) +def test_cli_max_skills_rejects_non_collection(tmp_path, monkeypatch, recursive): + monkeypatch.setattr(cli, "_scan_skill", lambda **_: pytest.fail("must not scan")) + args = ["scan", str(tmp_path), "--max-skills", "100"] + if recursive: + args.append("--recursive") + result = runner.invoke(app, args) + assert result.exit_code == 2 + assert "--max-skills" in result.stderr + + +def test_cli_max_skills_reaches_recursive_scan(tmp_path, monkeypatch): + for i in range(33): + skill = tmp_path / f"s{i:03}" + skill.mkdir() + (skill / "SKILL.md").write_text("---\nname: safe\n---\nHello\n") + calls = [] + + def fake_scan(**kwargs): + calls.append(kwargs) + return _bounded_recursive_result("safe", finding_count=0) + + monkeypatch.setattr(cli, "_scan_skill", fake_scan) + result = runner.invoke( + app, + [ + "scan", + str(tmp_path), + "--recursive", + "--max-skills", + "33", + "--no-llm", + "--format", + "json", + ], + ) + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout) + assert len(calls) == payload["skills_scanned"] == 33 + assert payload["skills_omitted"] == 0 + + +def test_cli_max_skills_is_documented_in_help(): + result = runner.invoke(app, ["scan", "--help"]) + assert result.exit_code == 0 + assert "--max-skills" in result.stdout + assert "32" in result.stdout + + +@pytest.mark.parametrize("limit", [0, -1, True, 1.5]) +def test_recursive_max_skills_helper_rejects_invalid_limit(tmp_path, limit): + detection = MultiSkillDetectionResult(is_multi_skill=True, skills=[]) + with pytest.raises(ValueError, match="positive integer"): + _scan_multi_skill(detection, FormatChoice.json, None, no_llm=True, max_skills=limit) + + +@pytest.mark.parametrize("max_skills", [None, 100]) def test_recursive_json_uses_one_global_public_record_budget( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, max_skills ) -> None: """Per-child report caps cannot multiply in the combined JSON document.""" skills = [SkillDirectory(tmp_path / name, name, name) for name in ("one", "two", "three")] @@ -2452,7 +2557,7 @@ def fake_invoke(*_args, **_kwargs) -> dict[str, object]: fake_invoke, ) - _scan_multi_skill(detection, FormatChoice.json, output, no_llm=True) + _scan_multi_skill(detection, FormatChoice.json, output, no_llm=True, max_skills=max_skills) assert len(calls) == 1 payload = json.loads(output.read_text(encoding="utf-8")) From d64edcd23bc778e67d50836ddfe81400bf1de344 Mon Sep 17 00:00:00 2001 From: Whj9283 <1621370123@qq.com> Date: Sat, 17 Oct 2026 10:09:34 +0800 Subject: [PATCH 2/2] test(cli): normalize ANSI styling in recursive limit help checks Signed-off-by: Whj9283 <1621370123@qq.com> --- tests/unit/test_cli.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 6f84dc450..2de29cda5 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -36,6 +36,7 @@ import pytest import typer import yaml +from click import unstyle from markdown_it import MarkdownIt from rich.console import Console from typer.testing import CliRunner @@ -2522,11 +2523,19 @@ def fake_scan(**kwargs): assert payload["skills_omitted"] == 0 -def test_cli_max_skills_is_documented_in_help(): - result = runner.invoke(app, ["scan", "--help"]) +@pytest.mark.parametrize("color", [False, True]) +def test_cli_max_skills_is_documented_in_help(monkeypatch, color): + # Typer initializes its terminal mode at import time, so environment-only + # changes here do not reliably exercise the GitHub Actions rendering path. + monkeypatch.delenv("NO_COLOR", raising=False) + monkeypatch.setattr("typer.rich_utils.FORCE_TERMINAL", color) + monkeypatch.setattr("typer.rich_utils.COLOR_SYSTEM", "standard" if color else None) + result = runner.invoke(app, ["scan", "--help"], color=True) assert result.exit_code == 0 - assert "--max-skills" in result.stdout - assert "32" in result.stdout + assert ("\x1b[" in result.stdout) is color + help_text = unstyle(result.stdout) + assert "--max-skills" in help_text + assert "32" in help_text @pytest.mark.parametrize("limit", [0, -1, True, 1.5])