diff --git a/CHANGELOG.md b/CHANGELOG.md index 224378b1..566f0952 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +### 2.9.2 (Monday, August 10, 2026) +### Features/Bug Fixes +* fix(llm): retry malformed structured responses +--- ### 2.9.1 (Monday, August 10, 2026) ### Features/Bug Fixes * fix(llm): add bounded connection retries diff --git a/docs/release/skillspector-2.9.2.md b/docs/release/skillspector-2.9.2.md new file mode 100644 index 00000000..27090d45 --- /dev/null +++ b/docs/release/skillspector-2.9.2.md @@ -0,0 +1,48 @@ +# SkillSpector v2.9.2 + +Released: 2026-08-10 + +## Summary + +This patch release makes structured LLM response handling more resilient to transient malformed payloads. It retries validation and structured-output parse failures with bounded backoff while retaining fail-closed batch isolation after the retry budget is exhausted. + +## Highlights + +- Structured LLM response failures now receive bounded retries before a batch is isolated. + +## Added + +- None. + +## Changed + +- Applied the structured-response retry policy centrally to semantic analyzers, the meta-analyzer, TP4, and gap-fill paths. + +## Fixed + +- Prevented transient malformed structured responses from immediately exhausting required LLM batches. + +## Security + +- None. + +## Breaking Changes and Migration + +- None. + +## Deprecations + +- None. + +## Validation + +- `git diff --check release/2.9.1..origin/main` — passed. +- Automated CI — lint, unit, integration, and Docker smoke checks passed; Sonar analysis succeeded. + +## Known Limitations + +- Requests still fail closed after the bounded retry budget is exhausted. + +## References + +- `CHANGELOG.md` diff --git a/pyproject.toml b/pyproject.toml index 7c7be63b..fe27d95b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "skillspector" -version = "2.9.1" +version = "2.9.2" description = "SkillSpector: Security scanner for AI agent skills (Claude Code, Cursor, and similar). Scans skills for vulnerabilities, malicious patterns, and security risks before installation. Supports Git repos, URLs, zips, and local directories; runs static pattern checks and optional LLM semantic analysis; outputs terminal, JSON, and Markdown reports with risk scoring." readme = "README.md" license = "Apache-2.0" diff --git a/src/skillspector/inspection_ledger.py b/src/skillspector/inspection_ledger.py index a0c286b8..00482a71 100644 --- a/src/skillspector/inspection_ledger.py +++ b/src/skillspector/inspection_ledger.py @@ -78,7 +78,7 @@ class LedgerReason(StrEnum): LedgerReason.SYNTAX_ERROR: "Python source could not be parsed.", LedgerReason.LLM_BATCH_FAILED: "LLM analysis failed for this file range.", LedgerReason.LLM_STRUCTURED_RESPONSE_INVALID: ( - "LLM returned a malformed structured response after retry." + "LLM returned a malformed structured response after bounded retries." ), LedgerReason.LLM_CONNECTION_RETRIES_EXHAUSTED: ("LLM connection failed after bounded retries."), LedgerReason.ANALYZER_RUNTIME_ERROR: ("Analyzer failed after beginning applicable work."), diff --git a/src/skillspector/llm_analyzer_base.py b/src/skillspector/llm_analyzer_base.py index ec419e32..0420ff3d 100644 --- a/src/skillspector/llm_analyzer_base.py +++ b/src/skillspector/llm_analyzer_base.py @@ -62,9 +62,11 @@ logger = get_logger(__name__) DEFAULT_MAX_LLM_CONCURRENCY = 10 -STRUCTURED_RESPONSE_MAX_ATTEMPTS = 2 API_CONNECTION_MAX_RETRIES = 3 API_CONNECTION_RETRY_DELAYS_SECONDS = (0.5, 1.0, 2.0) +STRUCTURED_RESPONSE_MAX_RETRIES = 3 +STRUCTURED_RESPONSE_MAX_ATTEMPTS = STRUCTURED_RESPONSE_MAX_RETRIES + 1 +STRUCTURED_RESPONSE_RETRY_DELAYS_SECONDS = API_CONNECTION_RETRY_DELAYS_SECONDS LLM_BATCH_MAX_ATTEMPTS = STRUCTURED_RESPONSE_MAX_ATTEMPTS + API_CONNECTION_MAX_RETRIES @@ -624,11 +626,16 @@ def _invoke_batch_with_retries(self, batch: Batch, prompt: str) -> tuple[Batch, or attempt == LLM_BATCH_MAX_ATTEMPTS ): raise + delay = STRUCTURED_RESPONSE_RETRY_DELAYS_SECONDS[structured_retries] structured_retries += 1 logger.warning( - "LLM structured response validation failed for %s; retrying once", + "LLM structured response validation failed for %s; retrying in %.2fs (%d/%d)", batch.file_label, + delay, + structured_retries, + STRUCTURED_RESPONSE_MAX_RETRIES, ) + time.sleep(delay) except Exception as exc: if ( not _is_retryable_api_connection_error(exc) @@ -685,11 +692,16 @@ async def _ainvoke_batch_with_retries(self, batch: Batch, prompt: str) -> tuple[ or attempt == LLM_BATCH_MAX_ATTEMPTS ): raise + delay = STRUCTURED_RESPONSE_RETRY_DELAYS_SECONDS[structured_retries] structured_retries += 1 logger.warning( - "LLM structured response validation failed for %s; retrying once", + "LLM structured response validation failed for %s; retrying in %.2fs (%d/%d)", batch.file_label, + delay, + structured_retries, + STRUCTURED_RESPONSE_MAX_RETRIES, ) + await asyncio.sleep(delay) except Exception as exc: if ( not _is_retryable_api_connection_error(exc) @@ -793,8 +805,9 @@ async def arun_batches( native retry timing remains provider-managed. Unrecovered errors cost only their own batch and are omitted from the result. Malformed structured responses (Pydantic ``ValidationError`` or CLI - JSON parse failures) are retried once and then isolated to their batch. - A batch makes at most five outer chat-model invocations even when both + JSON parse failures) receive three bounded exponential-backoff retries + and are then isolated to their batch. A batch makes at most seven outer + chat-model invocations even when both retry policies apply; native provider retries can make additional HTTP requests within one invocation. Callers can detect partial results by comparing the returned batches diff --git a/tests/nodes/test_llm_analyzer_base.py b/tests/nodes/test_llm_analyzer_base.py index 3c980f08..30a3a9ba 100644 --- a/tests/nodes/test_llm_analyzer_base.py +++ b/tests/nodes/test_llm_analyzer_base.py @@ -20,6 +20,7 @@ import json from unittest.mock import AsyncMock, MagicMock, patch +import httpx import pytest from langchain_anthropic import ChatAnthropic from langchain_core.messages import AIMessage @@ -293,6 +294,66 @@ def test_chunk_offset_preserved(self) -> None: assert "lines 50" in prompt +# --------------------------------------------------------------------------- +# LLMAnalyzerBase structured-output configuration +# --------------------------------------------------------------------------- + + +class TestStructuredOutputConfiguration: + MODEL = "azure/anthropic/claude-sonnet-4-6" + + def test_chat_openai_pydantic_schema_serializes_as_strict_json_schema( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + payloads: list[dict] = [] + + def capture_request(request: httpx.Request) -> httpx.Response: + payloads.append(json.loads(request.content)) + return httpx.Response( + 200, + json={ + "id": "chatcmpl-test", + "object": "chat.completion", + "created": 0, + "model": self.MODEL, + "choices": [ + { + "index": 0, + "finish_reason": "stop", + "message": { + "role": "assistant", + "content": '{"findings": []}', + "refusal": None, + }, + } + ], + "usage": { + "prompt_tokens": 1, + "completion_tokens": 1, + "total_tokens": 2, + }, + }, + request=request, + ) + + client = httpx.Client(transport=httpx.MockTransport(capture_request)) + llm = ChatOpenAI( + model=self.MODEL, + api_key="test", + base_url="https://inference-api.nvidia.com/v1", + http_client=client, + ) + monkeypatch.setattr(MOCK_PATCH_TARGET, lambda **_kwargs: llm) + + analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) + response = analyzer._structured_llm.invoke("test") + + assert response == LLMAnalysisResult(findings=[]) + response_format = payloads[0]["response_format"]["json_schema"] + assert response_format["strict"] is True + assert response_format["schema"]["additionalProperties"] is False + + # --------------------------------------------------------------------------- # LLMAnalyzerBase.parse_response (default — returns Finding objects) # --------------------------------------------------------------------------- @@ -503,7 +564,8 @@ def test_native_openai_connection_errors_are_not_retried_by_coordinator( ] @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) - def test_structured_validation_error_recovers_on_retry(self) -> None: + @patch("skillspector.llm_analyzer_base.time.sleep") + def test_structured_validation_error_recovers_on_retry(self, sleep: MagicMock) -> None: analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) analyzer._structured_llm.invoke = MagicMock( side_effect=[ @@ -518,12 +580,40 @@ def test_structured_validation_error_recovers_on_retry(self) -> None: assert [item[0].file_path for item in outcome.successful] == ["a.py"] assert outcome.failures == [] assert analyzer._structured_llm.invoke.call_count == 2 + sleep.assert_called_once_with(0.5) @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) - def test_structured_parse_error_recovers_on_retry(self) -> None: + @patch("skillspector.llm_analyzer_base.time.sleep") + def test_structured_validation_error_recovers_on_third_retry(self, sleep: MagicMock) -> None: + analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) + analyzer._structured_llm.invoke = MagicMock( + side_effect=[ + _structured_response_validation_error(), + _structured_response_validation_error(), + _structured_response_validation_error(), + LLMAnalysisResult(findings=[]), + ] + ) + + outcome = analyzer.run_batches_detailed([Batch(file_path="a.py", content="code")]) + + assert [item[0].file_path for item in outcome.successful] == ["a.py"] + assert outcome.failures == [] + assert analyzer._structured_llm.invoke.call_count == 4 + assert sleep.call_args_list == [ + ((0.5,), {}), + ((1.0,), {}), + ((2.0,), {}), + ] + + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) + @patch("skillspector.llm_analyzer_base.time.sleep") + def test_structured_parse_error_recovers_on_third_retry(self, sleep: MagicMock) -> None: analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) analyzer._structured_llm.invoke = MagicMock( side_effect=[ + StructuredOutputParseError("could not extract JSON"), + StructuredOutputParseError("could not extract JSON"), StructuredOutputParseError("could not extract JSON"), LLMAnalysisResult(findings=[]), ] @@ -533,10 +623,16 @@ def test_structured_parse_error_recovers_on_retry(self) -> None: assert [item[0].file_path for item in outcome.successful] == ["a.py"] assert outcome.failures == [] - assert analyzer._structured_llm.invoke.call_count == 2 + assert analyzer._structured_llm.invoke.call_count == 4 + assert sleep.call_args_list == [ + ((0.5,), {}), + ((1.0,), {}), + ((2.0,), {}), + ] @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) - def test_cli_structured_parse_error_recovers_on_retry(self) -> None: + @patch("skillspector.llm_analyzer_base.time.sleep") + def test_cli_structured_parse_error_recovers_on_retry(self, sleep: MagicMock) -> None: analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) provider = MagicMock() provider.complete.side_effect = ["not JSON", '{"findings": []}'] @@ -548,12 +644,18 @@ def test_cli_structured_parse_error_recovers_on_retry(self) -> None: assert [item[0].file_path for item in outcome.successful] == ["a.py"] assert outcome.failures == [] assert provider.complete.call_count == 2 + sleep.assert_called_once_with(0.5) @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) - def test_structured_validation_error_isolated_after_retry(self) -> None: + @patch("skillspector.llm_analyzer_base.time.sleep") + def test_structured_validation_error_isolated_after_three_retries( + self, sleep: MagicMock + ) -> None: analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) analyzer._structured_llm.invoke = MagicMock( side_effect=[ + _structured_response_validation_error(), + _structured_response_validation_error(), _structured_response_validation_error(), _structured_response_validation_error(), LLMAnalysisResult(findings=[]), @@ -570,7 +672,12 @@ def test_structured_validation_error_isolated_after_retry(self) -> None: assert [(failure.batch.file_path, failure.error_class) for failure in outcome.failures] == [ ("malformed.py", "ValidationError") ] - assert analyzer._structured_llm.invoke.call_count == 3 + assert analyzer._structured_llm.invoke.call_count == 5 + assert sleep.call_args_list == [ + ((0.5,), {}), + ((1.0,), {}), + ((2.0,), {}), + ] @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) @patch("skillspector.llm_analyzer_base.time.sleep") @@ -641,6 +748,7 @@ def test_structured_error_then_connection_errors_keeps_both_retry_policies( assert outcome.failures == [] assert analyzer._structured_llm.invoke.call_count == 5 assert sleep.call_args_list == [ + ((0.5,), {}), ((0.5,), {}), ((1.0,), {}), ((2.0,), {}), @@ -727,7 +835,8 @@ async def test_detailed_outcome_preserves_failed_batch(self) -> None: assert outcome.failures[0].error_class == "TimeoutError" @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) - async def test_structured_validation_error_recovers_on_retry(self) -> None: + @patch("skillspector.llm_analyzer_base.asyncio.sleep", new_callable=AsyncMock) + async def test_structured_validation_error_recovers_on_retry(self, sleep: AsyncMock) -> None: analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) analyzer._structured_llm.ainvoke = AsyncMock( side_effect=[ @@ -742,12 +851,42 @@ async def test_structured_validation_error_recovers_on_retry(self) -> None: assert [item[0].file_path for item in outcome.successful] == ["a.py"] assert outcome.failures == [] assert analyzer._structured_llm.ainvoke.call_count == 2 + sleep.assert_awaited_once_with(0.5) @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) - async def test_structured_parse_error_recovers_on_retry(self) -> None: + @patch("skillspector.llm_analyzer_base.asyncio.sleep", new_callable=AsyncMock) + async def test_structured_validation_error_recovers_on_third_retry( + self, sleep: AsyncMock + ) -> None: analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) analyzer._structured_llm.ainvoke = AsyncMock( side_effect=[ + _structured_response_validation_error(), + _structured_response_validation_error(), + _structured_response_validation_error(), + LLMAnalysisResult(findings=[]), + ] + ) + + outcome = await analyzer.arun_batches_detailed([Batch(file_path="a.py", content="code")]) + + assert [item[0].file_path for item in outcome.successful] == ["a.py"] + assert outcome.failures == [] + assert analyzer._structured_llm.ainvoke.call_count == 4 + assert sleep.await_args_list == [ + ((0.5,), {}), + ((1.0,), {}), + ((2.0,), {}), + ] + + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) + @patch("skillspector.llm_analyzer_base.asyncio.sleep", new_callable=AsyncMock) + async def test_structured_parse_error_recovers_on_third_retry(self, sleep: AsyncMock) -> None: + analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) + analyzer._structured_llm.ainvoke = AsyncMock( + side_effect=[ + StructuredOutputParseError("could not extract JSON"), + StructuredOutputParseError("could not extract JSON"), StructuredOutputParseError("could not extract JSON"), LLMAnalysisResult(findings=[]), ] @@ -757,13 +896,23 @@ async def test_structured_parse_error_recovers_on_retry(self) -> None: assert [item[0].file_path for item in outcome.successful] == ["a.py"] assert outcome.failures == [] - assert analyzer._structured_llm.ainvoke.call_count == 2 + assert analyzer._structured_llm.ainvoke.call_count == 4 + assert sleep.await_args_list == [ + ((0.5,), {}), + ((1.0,), {}), + ((2.0,), {}), + ] @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) - async def test_structured_validation_error_isolated_after_retry(self) -> None: + @patch("skillspector.llm_analyzer_base.asyncio.sleep", new_callable=AsyncMock) + async def test_structured_validation_error_isolated_after_three_retries( + self, sleep: AsyncMock + ) -> None: analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) analyzer._structured_llm.ainvoke = AsyncMock( side_effect=[ + _structured_response_validation_error(), + _structured_response_validation_error(), _structured_response_validation_error(), _structured_response_validation_error(), LLMAnalysisResult(findings=[]), @@ -780,7 +929,12 @@ async def test_structured_validation_error_isolated_after_retry(self) -> None: assert [(failure.batch.file_path, failure.error_class) for failure in outcome.failures] == [ ("malformed.py", "ValidationError") ] - assert analyzer._structured_llm.ainvoke.call_count == 3 + assert analyzer._structured_llm.ainvoke.call_count == 5 + assert sleep.await_args_list == [ + ((0.5,), {}), + ((1.0,), {}), + ((2.0,), {}), + ] @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) @patch("skillspector.llm_analyzer_base.asyncio.sleep", new_callable=AsyncMock) @@ -874,6 +1028,7 @@ async def test_structured_error_then_connection_errors_keeps_both_retry_policies assert outcome.failures == [] assert analyzer._structured_llm.ainvoke.call_count == 5 assert sleep.await_args_list == [ + ((0.5,), {}), ((0.5,), {}), ((1.0,), {}), ((2.0,), {}), @@ -1136,7 +1291,10 @@ def test_safe_failure_reason_is_preserved_in_ledger_events(self) -> None: ) assert events[0]["reason_code"] == LedgerReason.LLM_STRUCTURED_RESPONSE_INVALID - assert events[0]["message"] == "LLM returned a malformed structured response after retry." + assert ( + events[0]["message"] + == "LLM returned a malformed structured response after bounded retries." + ) completeness, _ = finalize_ledger( { @@ -1151,7 +1309,7 @@ def test_safe_failure_reason_is_preserved_in_ledger_events(self) -> None: LedgerReason.LLM_STRUCTURED_RESPONSE_INVALID ) assert completeness["ledger_exceptions"][0]["message"] == ( - "LLM returned a malformed structured response after retry." + "LLM returned a malformed structured response after bounded retries." ) def test_successful_unchunked_retry_has_one_terminal_outcome(self) -> None: diff --git a/tests/test_mcp_tool_poisoning.py b/tests/test_mcp_tool_poisoning.py index 06357739..05b97989 100644 --- a/tests/test_mcp_tool_poisoning.py +++ b/tests/test_mcp_tool_poisoning.py @@ -709,18 +709,21 @@ def test_llm_call_failure_returns_empty(self, monkeypatch: pytest.MonkeyPatch): def test_persistently_malformed_response_returns_empty(self, monkeypatch: pytest.MonkeyPatch): state = _make_state("mcp_mismatched_skill", use_llm=True) + monkeypatch.setattr("skillspector.llm_analyzer_base.time.sleep", lambda _delay: None) structured_llm = _mock_tp4_structured_llm( monkeypatch, - [{}, {}], + [{}, {}, {}, {}], ) result = node(state) tp4 = [f for f in result["findings"] if f.rule_id == "TP4"] assert len(tp4) == 0 - assert structured_llm.calls == 2 + assert structured_llm.calls == 4 assert result["inspection_ledger"][1]["error_class"] == "ValidationError" def test_malformed_response_is_retried(self, monkeypatch: pytest.MonkeyPatch): state = _make_state("mcp_mismatched_skill", use_llm=True) + sleep = MagicMock() + monkeypatch.setattr("skillspector.llm_analyzer_base.time.sleep", sleep) structured_llm = _mock_tp4_structured_llm( monkeypatch, [{}, {"is_mismatch": False}], @@ -729,11 +732,14 @@ def test_malformed_response_is_retried(self, monkeypatch: pytest.MonkeyPatch): result = node(state) assert structured_llm.calls == 2 + sleep.assert_called_once_with(0.5) assert result["llm_call_log"] == [{"node": "mcp_tool_poisoning", "ok": True, "error": None}] assert result["analyzer_status_events"][0]["status"] == "completed" def test_cli_parse_error_is_retried(self, monkeypatch: pytest.MonkeyPatch): state = _make_state("mcp_mismatched_skill", use_llm=True) + sleep = MagicMock() + monkeypatch.setattr("skillspector.llm_analyzer_base.time.sleep", sleep) provider = MagicMock() provider.complete.side_effect = ["not JSON", '{"is_mismatch": false}'] monkeypatch.setattr( @@ -744,10 +750,13 @@ def test_cli_parse_error_is_retried(self, monkeypatch: pytest.MonkeyPatch): result = node(state) assert provider.complete.call_count == 2 + sleep.assert_called_once_with(0.5) assert result["llm_call_log"] == [{"node": "mcp_tool_poisoning", "ok": True, "error": None}] def test_out_of_range_confidence_is_retried(self, monkeypatch: pytest.MonkeyPatch): state = _make_state("mcp_mismatched_skill", use_llm=True) + sleep = MagicMock() + monkeypatch.setattr("skillspector.llm_analyzer_base.time.sleep", sleep) structured_llm = _mock_tp4_structured_llm( monkeypatch, [{"is_mismatch": True, "confidence": 1.7}, {"is_mismatch": False}], @@ -756,6 +765,7 @@ def test_out_of_range_confidence_is_retried(self, monkeypatch: pytest.MonkeyPatc result = node(state) assert structured_llm.calls == 2 + sleep.assert_called_once_with(0.5) assert [finding for finding in result["findings"] if finding.rule_id == "TP4"] == [] assert result["llm_call_log"] == [{"node": "mcp_tool_poisoning", "ok": True, "error": None}] diff --git a/uv.lock b/uv.lock index 9887fd8c..53992dd1 100644 --- a/uv.lock +++ b/uv.lock @@ -2675,7 +2675,7 @@ wheels = [ [[package]] name = "skillspector" -version = "2.9.1" +version = "2.9.2" source = { editable = "." } dependencies = [ { name = "boto3" },