From fde55d55542a9d14ce2fccda92a7089a2a5dd579 Mon Sep 17 00:00:00 2001
From: andrewwhitecdw <andrewwhitecdw@users.noreply.github.com>
Date: Fri, 21 Aug 2026 14:38:12 -0500
Subject: [PATCH] fix: normalize multi-skill risk_score before threshold check

`_scan_multi_skill` checked `isinstance(score, int)` before comparing a
skill's `risk_score` against the running maximum. When the score is stored as
a string (e.g., deserialized from JSON), the check failed and the value was
ignored, so the aggregate exit code did not reflect a high-risk skill.

Convert the score to `int` safely with a fallback to `0`, then compare it
unconditionally. Add regression tests covering numeric-string scores and the
malformed-value fallback.

Signed-off-by: andrewwhitecdw <andrewwhitecdw@users.noreply.github.com>
---
 src/skillspector/cli.py |  6 ++++-
 tests/unit/test_cli.py  | 54 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/src/skillspector/cli.py b/src/skillspector/cli.py
index 4bb3e34d..ee748f4c 100644
--- a/src/skillspector/cli.py
+++ b/src/skillspector/cli.py
@@ -2181,7 +2181,11 @@ def _scan_multi_skill(
             elif not child_failed:
                 complete_skill_count += 1
             score = result.get("risk_score") or 0
-            if isinstance(score, int) and score > max_score:
+            try:
+                score = int(score)
+            except (TypeError, ValueError):
+                score = 0
+            if score > max_score:
                 max_score = score
             child_transitive_count = result.get("transitive_finding_count")
             if isinstance(child_transitive_count, int):
diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py
index bbb62c6e..c4e9bf29 100644
--- a/tests/unit/test_cli.py
+++ b/tests/unit/test_cli.py
@@ -264,6 +264,60 @@ def test_recursive_scan_exception_marks_combined_execution_as_failed(tmp_path: P
     assert payload["skills"][1] == {"name": "two", "error": "child scan crashed"}
 
 
+def test_recursive_scan_string_risk_score_counts_toward_exit_code(tmp_path: Path) -> None:
+    """Numeric-string risk_score values are coerced and affect aggregate exit code."""
+    s1 = SkillDirectory(path=tmp_path / "low", name="low", relative_path="low")
+    s2 = SkillDirectory(path=tmp_path / "high", name="high", relative_path="high")
+    detection = MultiSkillDetectionResult(
+        is_multi_skill=True, skills=[s1, s2], has_root_skill=False
+    )
+    output = tmp_path / "combined.json"
+
+    with patch(
+        "skillspector.cli.graph.invoke",
+        side_effect=[
+            {"report_body": '{"skill": {"name": "low"}}', "risk_score": "25"},
+            {"report_body": '{"skill": {"name": "high"}}', "risk_score": "75"},
+        ],
+    ):
+        with pytest.raises(typer.Exit) as exit_info:
+            _scan_multi_skill(
+                detection,
+                FormatChoice.json,
+                output,
+                no_llm=True,
+                yara_rules_dir=None,
+                verbose=False,
+            )
+
+    assert exit_info.value.exit_code == 1
+    payload = json.loads(output.read_text())
+    assert payload["max_risk_score"] == 75
+
+
+def test_recursive_scan_malformed_risk_score_falls_back_to_zero(tmp_path: Path) -> None:
+    """Non-numeric risk_score values fall back to 0 and do not raise."""
+    s1 = SkillDirectory(path=tmp_path / "bad", name="bad", relative_path="bad")
+    detection = MultiSkillDetectionResult(is_multi_skill=True, skills=[s1], has_root_skill=False)
+    output = tmp_path / "combined.json"
+
+    with patch(
+        "skillspector.cli.graph.invoke",
+        return_value={"report_body": '{"skill": {"name": "bad"}}', "risk_score": "not-a-number"},
+    ):
+        _scan_multi_skill(
+            detection,
+            FormatChoice.json,
+            output,
+            no_llm=True,
+            yara_rules_dir=None,
+            verbose=False,
+        )
+
+    payload = json.loads(output.read_text())
+    assert payload["max_risk_score"] == 0
+
+
 def test_cli_scan_slack_p6_pe3_regression(tmp_path: Path) -> None:
     """Benign context stays distinguishable without deleting deterministic CLI evidence."""
     (tmp_path / "references").mkdir()