From 1f914d27db4021e78d77917c9632bb0f972349da Mon Sep 17 00:00:00 2001 From: Udaya Tejas Date: Sat, 19 Sep 2026 18:58:52 +0000 Subject: [PATCH] fix(analyzers): surface analyzer modules dropped at registry load time _discover_analyzers() logs an ImportError/Exception at ERROR level and continues when an analyzer module fails to import, but the module is then never registered in ANALYZER_NODE_IDS. graph.py only ever wires nodes for IDs in that list, so the dropped analyzer gets no graph node, runs no node(), and emits no inspection-ledger event of its own. Nothing in analysis_completeness can see the gap: the scan reports status "complete" and the recommendation stays SAFE having never run that analyzer. Record each load failure in a new ANALYZER_LOAD_ERRORS dict, and have finalize_inspection_ledger emit one SYSTEM/PARTIAL ledger event per entry (reason ANALYZER_LOAD_ERROR), following the same SYSTEM-record convention finalize_inspection_ledger.py already uses for the finding-output-limit case. This degrades analysis_completeness to "partial" without flipping execution_successful to False, so it does not trip cli.py's unconditional exit(2) for a real crash - a missing analyzer is a coverage gap, not an execution failure. Regression test simulates a load failure by monkeypatching ANALYZER_LOAD_ERRORS and asserts the scan reports partial/incomplete instead of clean; a companion test asserts the unaffected case is untouched. Signed-off-by: Udaya Tejas --- src/skillspector/inspection_ledger.py | 4 ++ src/skillspector/nodes/analyzers/__init__.py | 5 +- .../nodes/finalize_inspection_ledger.py | 32 +++++++++- .../nodes/test_finalize_inspection_ledger.py | 62 +++++++++++++++++++ 4 files changed, 100 insertions(+), 3 deletions(-) diff --git a/src/skillspector/inspection_ledger.py b/src/skillspector/inspection_ledger.py index 836f2d6cc..564ed48bb 100644 --- a/src/skillspector/inspection_ledger.py +++ b/src/skillspector/inspection_ledger.py @@ -56,6 +56,7 @@ class LedgerReason(StrEnum): LLM_STRUCTURED_RESPONSE_INVALID = "llm_structured_response_invalid" LLM_CONNECTION_RETRIES_EXHAUSTED = "llm_connection_retries_exhausted" ANALYZER_RUNTIME_ERROR = "analyzer_runtime_error" + ANALYZER_LOAD_ERROR = "analyzer_load_error" UNACCOUNTED_WORK = "unaccounted_work" SEMANTIC_RUNTIME_INCOMPLETE = "semantic_runtime_incomplete" FINDING_ACCOUNTING_ERROR = "finding_accounting_error" @@ -118,6 +119,9 @@ class LedgerReason(StrEnum): ), LedgerReason.LLM_CONNECTION_RETRIES_EXHAUSTED: ("LLM connection failed after bounded retries."), LedgerReason.ANALYZER_RUNTIME_ERROR: ("Analyzer failed after beginning applicable work."), + LedgerReason.ANALYZER_LOAD_ERROR: ( + "Analyzer module failed to load and never began any inspection work." + ), LedgerReason.UNACCOUNTED_WORK: ("Planned inspection work has no unique terminal outcome."), LedgerReason.SEMANTIC_RUNTIME_INCOMPLETE: ( "Requested semantic analysis did not produce complete per-source runtime telemetry." diff --git a/src/skillspector/nodes/analyzers/__init__.py b/src/skillspector/nodes/analyzers/__init__.py index c3dfc16ab..c56ea01b6 100644 --- a/src/skillspector/nodes/analyzers/__init__.py +++ b/src/skillspector/nodes/analyzers/__init__.py @@ -28,6 +28,7 @@ ANALYZER_NODE_IDS: list[str] = [] ANALYZER_NODES: dict[str, Any] = {} ANALYZER_MODULES: dict[str, Any] = {} +ANALYZER_LOAD_ERRORS: dict[str, str] = {} def _discover_analyzers() -> None: @@ -44,9 +45,11 @@ def _discover_analyzers() -> None: mod = importlib.import_module(full_module_name) except ImportError as exc: logger.error("Failed to import analyzer module %s: %s", module_name, exc) + ANALYZER_LOAD_ERRORS[module_name] = str(exc) continue except Exception as exc: logger.error("Error loading analyzer module %s: %s", module_name, exc) + ANALYZER_LOAD_ERRORS[module_name] = str(exc) continue analyzer_id = getattr(mod, "ANALYZER_ID", None) @@ -60,4 +63,4 @@ def _discover_analyzers() -> None: _discover_analyzers() -__all__ = ["ANALYZER_NODE_IDS", "ANALYZER_NODES", "ANALYZER_MODULES"] +__all__ = ["ANALYZER_LOAD_ERRORS", "ANALYZER_MODULES", "ANALYZER_NODE_IDS", "ANALYZER_NODES"] diff --git a/src/skillspector/nodes/finalize_inspection_ledger.py b/src/skillspector/nodes/finalize_inspection_ledger.py index 81ba1cc81..b065b0e6e 100644 --- a/src/skillspector/nodes/finalize_inspection_ledger.py +++ b/src/skillspector/nodes/finalize_inspection_ledger.py @@ -18,7 +18,7 @@ ledger_event, ) from skillspector.models import Finding -from skillspector.nodes.analyzers import ANALYZER_MODULES +from skillspector.nodes.analyzers import ANALYZER_LOAD_ERRORS, ANALYZER_MODULES from skillspector.semantic_runtime import ( has_semantic_runtime_event, semantic_runtime_intent, @@ -147,6 +147,27 @@ def _size_coverage_findings( return findings +def _analyzer_load_error_events() -> list[InspectionLedgerEvent]: + """Surface analyzer modules dropped by the registry before any node ran. + + ``_discover_analyzers`` logs an import failure and moves on, so a module + that cannot be imported never gets a graph node and never emits a + work-item event of its own. Recorded here as a SYSTEM record so a dropped + analyzer degrades ``analysis_completeness`` instead of leaving a scan + silently short of whatever that analyzer would have looked for. + """ + return [ + ledger_event( + outcome=LedgerOutcome.PARTIAL, + record_type=LedgerRecordType.SYSTEM, + phase="analyzer_registry", + path=f"analyzer_registry/{module_name}", + reason=LedgerReason.ANALYZER_LOAD_ERROR, + ) + for module_name in sorted(ANALYZER_LOAD_ERRORS) + ] + + def finalize_inspection_ledger(state: SkillspectorState) -> dict[str, object]: """Validate full internal facts and derive the public completeness projection.""" reference_findings = _reference_coverage_findings(state) @@ -195,6 +216,7 @@ def finalize_inspection_ledger(state: SkillspectorState) -> dict[str, object]: limit_findings=MAX_FINDING_OUTPUT_RECORDS, ) ) + load_error_events = _analyzer_load_error_events() merged_state["findings"] = all_findings merged_state["effective_finding_ids"] = [ *(state.get("effective_finding_ids") or []), @@ -218,6 +240,7 @@ def finalize_inspection_ledger(state: SkillspectorState) -> dict[str, object]: *reference_events, *output_events, *runtime_events, + *load_error_events, ] reference_statuses = ( [analyzer_status_for_events("reference_coverage", reference_events)] @@ -239,6 +262,11 @@ def finalize_inspection_ledger(state: SkillspectorState) -> dict[str, object]: "execution_successful": completeness["execution_successful"], "findings": coverage_findings, "effective_finding_ids": effective_finding_ids, - "inspection_ledger": [*reference_events, *output_events, *runtime_events], + "inspection_ledger": [ + *reference_events, + *output_events, + *runtime_events, + *load_error_events, + ], "analyzer_status_events": reference_statuses, } diff --git a/tests/nodes/test_finalize_inspection_ledger.py b/tests/nodes/test_finalize_inspection_ledger.py index a0c424959..34ad337e7 100644 --- a/tests/nodes/test_finalize_inspection_ledger.py +++ b/tests/nodes/test_finalize_inspection_ledger.py @@ -901,3 +901,65 @@ def broken_node(_state: SkillspectorState) -> AnalyzerNodeResponse: assert result["inspection_ledger"][0]["error_class"] == "RuntimeError" assert "provider detail" not in result["inspection_ledger"][0]["message"] assert result["analyzer_status_events"][0]["status"] == "failed" + + +def test_analyzer_registry_load_failure_marks_scan_incomplete( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A module the registry dropped at import time must not report a clean scan. + + ``_discover_analyzers`` never wires a node for a module it fails to import, + so nothing else in the graph would otherwise notice that analyzer is + missing: no ledger event, no analyzer_status_events entry, no limitation. + """ + monkeypatch.setattr( + finalizer_module, + "ANALYZER_LOAD_ERRORS", + {"static_patterns_data_exfiltration": "ImportError: no module named 'yara'"}, + ) + + result = finalize_inspection_ledger( + { + "components": ["SKILL.md"], + "findings": [], + "effective_finding_ids": [], + "inspection_ledger": [], + "analyzer_status_events": [], + } + ) + + load_error_events = [ + event + for event in result["inspection_ledger"] + if event.get("reason_code") == LedgerReason.ANALYZER_LOAD_ERROR + ] + assert len(load_error_events) == 1 + assert load_error_events[0]["record_type"] == LedgerRecordType.SYSTEM + assert load_error_events[0]["outcome"] == LedgerOutcome.PARTIAL + assert load_error_events[0]["path"] == "analyzer_registry/static_patterns_data_exfiltration" + + completeness = result["analysis_completeness"] + assert completeness["status"] == "partial" + assert completeness["is_complete"] is False + # A dropped analyzer is a coverage gap, not an execution crash: the run + # must not be forced into `cli.py`'s unconditional exit(2) for + # execution_successful is False. + assert completeness["execution_successful"] is True + + +def test_no_analyzer_load_errors_leaves_completeness_untouched() -> None: + monkeypatch_free_result = finalize_inspection_ledger( + { + "components": ["SKILL.md"], + "findings": [], + "effective_finding_ids": [], + "inspection_ledger": [], + "analyzer_status_events": [], + } + ) + + assert monkeypatch_free_result["analysis_completeness"]["status"] == "complete" + assert not any( + event.get("reason_code") == LedgerReason.ANALYZER_LOAD_ERROR + for event in monkeypatch_free_result["inspection_ledger"] + )