Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/skillspector/nodes/finalize_inspection_ledger.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
}
_MAX_PASSIVE_PNG_CHUNKS = 4_096
_MAX_REFERENCE_REASONS = 16
_REFERENCE_CAVEAT_REASONS = (LedgerReason.REFERENCE_MISSING, LedgerReason.REFERENCE_UNRESOLVED)
_REFERENCE_LIMIT_FIELDS = frozenset(
f"{prefix}_{unit}"
for prefix in ("observed", "limit")
Expand Down Expand Up @@ -531,6 +532,18 @@ def _reference_coverage_findings(
else []
)
for event in ledger_events:
# Reference resolution records a mention of a path that the bundle does
# not carry, or that matches more than one bundled file, as a partial
# event on the file that contains the mention. Neither leaves bytes of
# that file uninspected. The completeness ledger still reports them;
# they must not turn a reference to that file (such as `SKILL.md`)
# into AE1.
if (
event.get("outcome") == LedgerOutcome.PARTIAL
and event.get("phase") == "reference_resolution"
and str(event.get("reason_code")) in _REFERENCE_CAVEAT_REASONS
):
continue
events_by_path.setdefault(str(event.get("path", "")), []).append(event)
status_shape_complete, invalid_status_paths = _status_paths_with_incomplete_evidence(
state.get("analyzer_status_events"), ledger_events
Expand Down
134 changes: 134 additions & 0 deletions tests/nodes/test_finalize_inspection_ledger.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import skillspector.nodes.report as report_module
import skillspector.state as state_module
from skillspector.inspection_ledger import (
InspectionLedgerEvent,
LedgerOutcome,
LedgerReason,
LedgerRecordType,
Expand Down Expand Up @@ -1907,6 +1908,139 @@ def test_unresolved_reference_does_not_synthesize_ae1(status: str) -> None:
assert result["effective_finding_ids"] == []


def _reference_caveat(
reason: LedgerReason,
*,
outcome: LedgerOutcome = LedgerOutcome.PARTIAL,
phase: str = "reference_resolution",
) -> InspectionLedgerEvent:
"""Build the event reference resolution records on SKILL.md for an unresolved mention."""
return ledger_event(
outcome=outcome,
record_type=LedgerRecordType.SYSTEM,
phase=phase,
path="SKILL.md",
start_line=3,
end_line=3,
reason=reason,
)


def _self_reference_state(
*events: InspectionLedgerEvent, status: str = "missing"
) -> SkillspectorState:
"""SKILL.md mentions an unresolved path and then references itself."""
return {
"artifact_inventory": [{"path": "SKILL.md", "disposition": "analyzed"}],
"artifact_references": [
{
"source_path": "SKILL.md",
"line": 3,
"evidence": "templates/config.yaml",
"target_path": None,
"status": status,
},
{
"source_path": "SKILL.md",
"line": 5,
"evidence": "Keep `SKILL.md` under 500 lines.",
"target_path": "SKILL.md",
"status": "resolved",
"disposition": "analyzed",
},
],
"inspection_ledger": list(events),
}


_REFERENCE_CAVEAT_CASES = [
pytest.param("missing", LedgerReason.REFERENCE_MISSING, id="missing"),
pytest.param("ambiguous", LedgerReason.REFERENCE_UNRESOLVED, id="ambiguous"),
]


@pytest.mark.parametrize(("status", "reason"), _REFERENCE_CAVEAT_CASES)
def test_reference_caveat_does_not_make_its_source_an_ae1_target(
status: str, reason: LedgerReason
) -> None:
state = _self_reference_state(_reference_caveat(reason), status=status)

assert finalizer_module._reference_coverage_findings(state) == []


@pytest.mark.parametrize(("status", "reason"), _REFERENCE_CAVEAT_CASES)
def test_self_reference_ae1_still_reports_source_content_limitations(
status: str, reason: LedgerReason
) -> None:
findings = finalizer_module._reference_coverage_findings(
_self_reference_state(
_reference_caveat(reason),
ledger_event(
outcome=LedgerOutcome.PARTIAL,
phase="static",
analyzer_id="static_patterns_tool_misuse",
path="SKILL.md",
reason=LedgerReason.STATIC_PARSE_LIMIT,
start_line=4,
end_line=4,
),
status=status,
)
)

assert [(finding.rule_id, finding.start_line) for finding in findings] == [("AE1", 5)]
reasons = findings[0].to_dict()["evidence"]["reasons"]
assert [row["reason_code"] for row in reasons] == ["static_parse_limit"]


@pytest.mark.parametrize(
("outcome", "phase", "reason"),
[
pytest.param(
LedgerOutcome.FAILED,
"reference_resolution",
LedgerReason.REFERENCE_MISSING,
id="failed-missing",
),
pytest.param(
LedgerOutcome.FAILED,
"reference_resolution",
LedgerReason.REFERENCE_UNRESOLVED,
id="failed-unresolved",
),
pytest.param(
LedgerOutcome.PARTIAL,
"static",
LedgerReason.REFERENCE_MISSING,
id="other-phase-missing",
),
pytest.param(
LedgerOutcome.PARTIAL,
"cache",
LedgerReason.REFERENCE_UNRESOLVED,
id="other-phase-unresolved",
),
pytest.param(
LedgerOutcome.PARTIAL,
"reference_resolution",
LedgerReason.REFERENCE_EXTRACTION_LIMIT,
id="extraction-limit",
),
],
)
def test_other_reference_event_shapes_still_yield_self_reference_ae1(
outcome: LedgerOutcome, phase: str, reason: LedgerReason
) -> None:
findings = finalizer_module._reference_coverage_findings(
_self_reference_state(_reference_caveat(reason, outcome=outcome, phase=phase))
)

assert [(finding.rule_id, finding.start_line) for finding in findings] == [("AE1", 5)]
evidence = findings[0].to_dict()["evidence"]
assert evidence["target_disposition"] == outcome.value
assert [row["reason_code"] for row in evidence["reasons"]] == [reason.value]


def test_guard_analyzer_node_converts_unexpected_exception_to_fatal_facts() -> None:
def broken_node(_state: SkillspectorState) -> AnalyzerNodeResponse:
raise RuntimeError("provider detail must remain private")
Expand Down
83 changes: 83 additions & 0 deletions tests/nodes/test_security_remediation.py
Original file line number Diff line number Diff line change
Expand Up @@ -1732,6 +1732,89 @@ def test_unresolved_primary_reference_blocks_complete_verdict(tmp_path: Path, ca
assert result["risk_recommendation"] != "SAFE"


@pytest.mark.parametrize(
("primary", "mentions", "status", "reason"),
[
pytest.param(
"SKILL.md",
"Other skills keep templates under `templates/config.yaml`.\n"
"Keep `SKILL.md` under 500 lines.\n",
"missing",
"reference_missing",
id="backticked-self-reference",
),
pytest.param(
"SKILL.md",
"Other skills keep templates under `templates/config.yaml`.\n"
"Keep [this file](SKILL.md) under 500 lines.\n",
"missing",
"reference_missing",
id="markdown-link-self-reference",
),
pytest.param(
"SKILL.md",
"Other skills keep templates in [config](templates/config.yaml).\n"
"Keep `SKILL.md` under 500 lines.\n",
"missing",
"reference_missing",
id="markdown-link-missing-target",
),
pytest.param(
"skill.md",
"Other skills keep templates under `templates/config.yaml`.\n"
"Keep `skill.md` under 500 lines.\n",
"missing",
"reference_missing",
id="lowercase-primary",
),
pytest.param(
"SKILL.md",
"Edit `config.yaml` before running.\nKeep `SKILL.md` under 500 lines.\n",
"ambiguous",
"reference_unresolved",
id="ambiguous-reference",
),
],
)
def test_reference_caveat_does_not_turn_self_reference_into_ae1(
tmp_path: Path, primary: str, mentions: str, status: str, reason: str
) -> None:
references = tmp_path / "references"
references.mkdir()
(references / "guide.md").write_text("# Guide\n\nPlain guide text.\n", encoding="utf-8")
if status == "ambiguous":
for subdirectory in ("a", "b"):
(tmp_path / subdirectory).mkdir()
(tmp_path / subdirectory / "config.yaml").write_text("mode: local\n", encoding="utf-8")
(tmp_path / primary).write_text(
f"# Skill\n\nRead [the guide](references/guide.md).\n{mentions}",
encoding="utf-8",
)

result = graph.invoke(
{
"input_path": str(tmp_path),
"output_format": "json",
"use_llm": False,
}
)

statuses = Counter(
(reference["status"], reference["target_path"])
for reference in result["artifact_references"]
)
assert statuses[("resolved", primary)] == 1
assert statuses[(status, None)] == 1
assert not any(finding.rule_id == "AE1" for finding in result["filtered_findings"])
# The unresolved mention is still reported as a completeness caveat.
assert [
(row["path"], row["reason_code"])
for row in result["analysis_completeness"]["ledger_exceptions"]
] == [(primary, reason)]
assert result["analysis_completeness"]["is_complete"] is False
assert result["risk_recommendation"] == "CAUTION"


@pytest.mark.asyncio
async def test_unresolved_reference_caveat_does_not_block_mcp_install(tmp_path: Path) -> None:
"""A reference caveat hides no bytes, so it must not fail safe_to_install.
Expand Down
Loading