Skip to content
Open
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
20 changes: 16 additions & 4 deletions src/skillspector/nodes/analyzers/static_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ def _is_env_file_reference_in_docs(
return False
if file_type not in ("markdown", "text"):
return False
if file_path.replace("\\", "/").lower().endswith("skill.md"):
if _is_skill_md(file_path):
return False
if not finding.context:
return False
Expand All @@ -165,6 +165,17 @@ def _is_eval_dataset(path: str) -> bool:
return path.replace("\\", "/") in _EVAL_DATASET_FILES


def _is_skill_md(path: str) -> bool:
"""Return True for paths with the existing loose SKILL.md suffix match."""
return path.lower().endswith("skill.md")


def _is_canonical_skill_md(path: str) -> bool:
"""Return True for files literally named SKILL.md."""
normalized = path.replace("\\", "/")
return normalized.rsplit("/", 1)[-1].lower() == "skill.md"


_DOCUMENTATION_DIR_NAMES = (
"docs",
"documentation",
Expand Down Expand Up @@ -192,7 +203,7 @@ def _is_documentation_context(af: AnalyzerFinding, file_type: str, path: str, co
"""Return true when a governed finding is prose or a comment without execution signals."""
if af.rule_id not in _SEMANTIC_STRING_DOC_PRONE_RULES:
return False
if path.replace("\\", "/").lower().endswith("skill.md"):
if _is_skill_md(path):
return False
lines = content.splitlines()
matched_line = (
Expand All @@ -212,7 +223,7 @@ def _is_documentation_markdown(path: str) -> bool:
normalized = path.replace("\\", "/").lower()
if not normalized.endswith((".md", ".markdown")):
return False
if normalized.endswith("skill.md"):
if _is_skill_md(path):
return False
parts = normalized.split("/")
return any(part in _DOCUMENTATION_DIR_NAMES for part in parts[:-1])
Expand Down Expand Up @@ -286,6 +297,7 @@ def run_static_patterns(
file_type = _infer_file_type(path)
is_doc_markdown = _is_documentation_markdown(path)
is_non_executable = file_type in _NON_EXECUTABLE_FILE_TYPES
is_canonical_skill_md = _is_canonical_skill_md(path)
for module in pattern_modules:
raw = module.analyze(content=content, file_path=path, file_type=file_type)
for af in raw:
Expand All @@ -297,7 +309,7 @@ def run_static_patterns(
af.location.start_line,
)
continue
if af.context and is_code_example(af.context):
if af.context and is_code_example(af.context) and not is_canonical_skill_md:
if is_non_executable:
logger.debug(
"Filtered code-example finding in non-executable: %s in %s:%d",
Expand Down
50 changes: 43 additions & 7 deletions tests/nodes/analyzers/test_static_runner_filtering.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ def test_extensionless_file_not_hard_dropped_by_code_example(self) -> None:
)

def test_skill_md_findings_are_not_filtered_by_backticks(self) -> None:
"""SKILL.md is the primary instruction file — backticks alone shouldn't filter."""
"""SKILL.md is the primary instruction file, so fenced findings survive."""
content = """\
---
name: deploy-tool
Expand All @@ -229,12 +229,48 @@ def test_skill_md_findings_are_not_filtered_by_backticks(self) -> None:
"file_cache": {"SKILL.md": content},
}
findings = static_runner.run_static_patterns(state, [tm_module])
# SKILL.md code blocks do get filtered by is_code_example (same as EA2/MP)
# This is correct: the meta-analyzer handles SKILL.md nuance
# The key test is that SKILL.md is NOT treated as documentation-path markdown
for f in findings:
# Confidence should NOT be reduced by _DOCUMENTATION_CONFIDENCE_FACTOR
assert f.confidence >= 0.3
tm1_findings = [f for f in findings if f.rule_id == "TM1"]
assert len(tm1_findings) == 1
assert tm1_findings[0].confidence == 0.6

def test_fenced_guide_md_findings_are_filtered(self) -> None:
"""Ordinary fenced Markdown remains filtered as a documentation example."""
content = """\
Use this tool to deploy:
```
curl -k https://production.example.com/deploy
```
"""
assert "TM1" not in _findings(content, "guide.md", tm_module)

def test_fenced_noncanonical_skill_md_suffix_paths_remain_filtered(self) -> None:
"""Only files literally named SKILL.md bypass the runner's code-example filter."""
content = """\
Use this tool to deploy:
```
curl -k https://production.example.com/deploy
```
"""
for path in ("my_skill.md", "docs/bash_skill.md"):
assert "TM1" not in _findings(content, path, tm_module)

@pytest.mark.parametrize(
("path", "expected_loose", "expected_canonical"),
[
("SKILL.md", True, True),
("nested/SKILL.md", True, True),
("nested\\skill.md", True, True),
("my_skill.md", True, False),
("docs/bash_skill.md", True, False),
("skill.md.bak", False, False),
],
)
def test_skill_md_path_helpers_distinguish_loose_and_canonical_matches(
self, path: str, expected_loose: bool, expected_canonical: bool
) -> None:
"""Legacy suffix checks stay loose, while the new runner gate is exact-basename."""
assert static_runner._is_skill_md(path) is expected_loose
assert static_runner._is_canonical_skill_md(path) is expected_canonical


class TestDocumentationPathConfidenceReduction:
Expand Down
27 changes: 27 additions & 0 deletions tests/unit/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,33 @@ def test_cli_scan_no_llm(tmp_path: Path) -> None:
assert result.exit_code == 0


def test_cli_scan_no_llm_reports_fenced_skill_md_tool_misuse(tmp_path: Path) -> None:
"""No-LLM scans retain TM1 from fenced SKILL.md instructions."""
(tmp_path / "SKILL.md").write_text(
"""---
name: deploy-tool
---
# Deploy Tool

Use this tool to deploy:
```
curl -k https://production.example.com/deploy
```

The agent will execute the above command.
""",
encoding="utf-8",
)

result = runner.invoke(app, ["scan", str(tmp_path), "--no-llm", "--format", "json"])

assert result.exit_code == 0
data = json.loads(result.output)
assert any(issue["id"] == "TM1" for issue in data["issues"])
assert data["analysis_completeness"]["findings_before_filtering"] > 0
assert data["analysis_completeness"]["findings_after_filtering"] > 0


def test_cli_scan_nonexistent_exits_2() -> None:
"""scan with nonexistent path exits with code 2."""
result = runner.invoke(app, ["scan", "/nonexistent/path/xyz"])
Expand Down