Files
jon 5162141430 fix(plugin-eval): exempt slash-only and path-triggered skills from MISSING_TRIGGER (#531)
* fix(plugin-eval): exempt slash-only and path-triggered skills from MISSING_TRIGGER

Skills can be invoked through three mechanisms in Claude Code:

1. Model-driven auto-invocation based on the description (the default,
   and what MISSING_TRIGGER is designed to gate).
2. User-driven slash invocation only — opt-out via
   `disable-model-invocation: true` in the SKILL.md frontmatter.
3. Path-triggered auto-load — opt-in via the `paths:` frontmatter glob,
   where the skill is loaded when the model opens a matching file.

The current `MISSING_TRIGGER` check runs against every skill regardless
of invocation mechanism. Skills in (2) cannot be auto-invoked at all, so
a trigger phrase in the description is irrelevant. Skills in (3) are
triggered by file paths, not description matching, so the description
serves as documentation rather than as a discovery surface.

Add a `_skill_uses_description_trigger(skill)` predicate that consults
`skill.frontmatter` and short-circuits the MISSING_TRIGGER check for
the two non-description invocation paths. Behaviour for ordinary
model-invocable skills is unchanged.

Tests:
- `test_disable_model_invocation_exempts_skill` — slash-only, no trigger,
  no flag.
- `test_paths_auto_load_exempts_skill` — path-triggered, no trigger,
  no flag.
- `test_disable_model_invocation_false_still_checks_trigger` — explicit
  `false` does not exempt.
- `test_empty_paths_value_still_checks_trigger` — empty string is not
  a valid auto-load configuration.

Full plugin-eval suite (73 tests) passes.

* fix: address ruff SIM103/SIM102 and guard paths check against non-string values

Simplify conditional returns (SIM103), collapse nested ifs (SIM102),
and use isinstance(paths, str) to prevent paths: [] from incorrectly
exempting a skill from MISSING_TRIGGER.

---------

Co-authored-by: Seth Hobson <wshobson@gmail.com>
2026-05-25 21:14:51 -04:00

208 lines
8.1 KiB
Python

from pathlib import Path
import pytest
from plugin_eval.layers.static import _TRIGGER_PATTERN, StaticAnalyzer
from plugin_eval.models import LayerResult
def _make_skill(tmp_path: Path, description: str, name: str = "test-skill") -> Path:
skill_dir = tmp_path / name
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
f"---\nname: {name}\ndescription: \"{description}\"\n---\n\n"
"# Skill\n\n## Overview\n\nBody.\n"
)
return skill_dir
class TestStaticAnalyzer:
def test_analyze_valid_skill(self, sample_skill_dir: Path):
analyzer = StaticAnalyzer()
result = analyzer.analyze_skill(sample_skill_dir)
assert isinstance(result, LayerResult)
assert result.layer == "static"
assert result.score > 0.5
assert len(result.anti_patterns) == 0
def test_analyze_poor_skill(self, poor_skill_dir: Path):
analyzer = StaticAnalyzer()
result = analyzer.analyze_skill(poor_skill_dir)
assert result.score < 0.7
flags = [ap.flag for ap in result.anti_patterns]
assert "OVER_CONSTRAINED" in flags
assert "MISSING_TRIGGER" in flags
def test_analyze_plugin(self, sample_plugin_dir: Path):
analyzer = StaticAnalyzer()
result = analyzer.analyze_plugin(sample_plugin_dir)
assert result.layer == "static"
assert result.score > 0.5
assert "skill_scores" in result.sub_scores
assert "agent_scores" in result.sub_scores
def test_anti_pattern_penalty(self):
analyzer = StaticAnalyzer()
assert analyzer._anti_pattern_penalty(0) == 1.0
assert analyzer._anti_pattern_penalty(2) == pytest.approx(0.9)
assert analyzer._anti_pattern_penalty(10) == 0.5
assert analyzer._anti_pattern_penalty(20) == 0.5
def test_description_pushiness_score(self):
analyzer = StaticAnalyzer()
good = "Test skill for evaluation. Use when testing plugin-eval. Use PROACTIVELY for quality checks."
weak = "A skill."
assert analyzer._description_pushiness(good) > analyzer._description_pushiness(weak)
class TestTriggerPattern:
"""Regression coverage for the broadened trigger-phrase matcher.
plugin-dev's canonical recommendation is third-person ("This skill should be
used when …"), and several real-world plugins use prepositional triggers
("Use after …", "Use before …"). The pre-2026 regex only matched the
imperative "Use when …" form, which produced false-positive MISSING_TRIGGER
flags against Anthropic's own examples.
"""
@pytest.mark.parametrize(
"description",
[
"Use when testing plugin-eval.",
"Use this skill when scaffolding plugins.",
"This skill should be used when the user asks to 'create a hook'.",
"Used when several attempts have failed in a row.",
"Use after editing the source-of-truth files, before committing.",
"Use before declaring a task complete after a hard debugging session.",
"Use immediately before a commit, push, or edit-after-failure.",
"Use whenever you are asked to plan inside a Paperclip company.",
"Auto-loads when working on test files.",
"Trigger when a Bash command fails three times in a row.",
"Use PROACTIVELY before merging.",
],
)
def test_pattern_matches_canonical_forms(self, description: str) -> None:
assert _TRIGGER_PATTERN.search(description), (
f"Expected trigger phrase to match in: {description!r}"
)
@pytest.mark.parametrize(
"description",
[
"A skill.",
"Provides hook guidance.",
"Returns the current timestamp.",
"Performs static analysis on plugin directories.",
],
)
def test_pattern_rejects_descriptions_without_trigger(self, description: str) -> None:
assert not _TRIGGER_PATTERN.search(description), (
f"Did not expect trigger match in: {description!r}"
)
def test_third_person_skill_does_not_flag_missing_trigger(self, tmp_path: Path) -> None:
"""Anthropic plugin-dev's canonical phrasing must not be flagged."""
skill_dir = _make_skill(
tmp_path,
"This skill should be used when the user asks to 'create a hook', "
"'add a PreToolUse hook', or 'validate tool use'.",
)
analyzer = StaticAnalyzer()
result = analyzer.analyze_skill(skill_dir)
flags = [ap.flag for ap in result.anti_patterns]
assert "MISSING_TRIGGER" not in flags
def test_prepositional_trigger_does_not_flag_missing_trigger(self, tmp_path: Path) -> None:
skill_dir = _make_skill(
tmp_path,
"Self-check before a single risky action. Use immediately before a "
"commit, push, edit-after-failure, or skip-a-verification step.",
)
analyzer = StaticAnalyzer()
result = analyzer.analyze_skill(skill_dir)
flags = [ap.flag for ap in result.anti_patterns]
assert "MISSING_TRIGGER" not in flags
def _make_skill_with_frontmatter(
tmp_path: Path, frontmatter_lines: list[str], name: str = "test-skill"
) -> Path:
skill_dir = tmp_path / name
skill_dir.mkdir()
frontmatter = "\n".join(frontmatter_lines)
(skill_dir / "SKILL.md").write_text(
f"---\n{frontmatter}\n---\n\n# Skill\n\n## Overview\n\nBody.\n"
)
return skill_dir
class TestTriggerExemptions:
"""`disable-model-invocation: true` and `paths:` frontmatter should exempt
a skill from the MISSING_TRIGGER check, because those skills are not
auto-invoked from the description.
"""
def test_disable_model_invocation_exempts_skill(self, tmp_path: Path) -> None:
skill_dir = _make_skill_with_frontmatter(
tmp_path,
[
"name: setup",
"description: One-time setup that adds .claude/state/ to the project's .gitignore.",
"disable-model-invocation: true",
],
)
analyzer = StaticAnalyzer()
result = analyzer.analyze_skill(skill_dir)
flags = [ap.flag for ap in result.anti_patterns]
assert "MISSING_TRIGGER" not in flags, (
"Slash-only skills should not be flagged for missing description trigger"
)
def test_paths_auto_load_exempts_skill(self, tmp_path: Path) -> None:
skill_dir = _make_skill_with_frontmatter(
tmp_path,
[
"name: self-evaluate",
"description: Self-critical evaluation guard for test/spec files.",
'paths: "**/*test*,**/*spec*"',
],
)
analyzer = StaticAnalyzer()
result = analyzer.analyze_skill(skill_dir)
flags = [ap.flag for ap in result.anti_patterns]
assert "MISSING_TRIGGER" not in flags, (
"Path-triggered skills should not be flagged for missing description trigger"
)
def test_disable_model_invocation_false_still_checks_trigger(
self, tmp_path: Path
) -> None:
skill_dir = _make_skill_with_frontmatter(
tmp_path,
[
"name: model-invocable",
"description: A description without a trigger phrase whatsoever.",
"disable-model-invocation: false",
],
)
analyzer = StaticAnalyzer()
result = analyzer.analyze_skill(skill_dir)
flags = [ap.flag for ap in result.anti_patterns]
assert "MISSING_TRIGGER" in flags, (
"Model-invocable skills without a trigger phrase must still be flagged"
)
def test_empty_paths_value_still_checks_trigger(self, tmp_path: Path) -> None:
skill_dir = _make_skill_with_frontmatter(
tmp_path,
[
"name: bad-paths",
"description: Some skill description that lacks the trigger phrase.",
'paths: ""',
],
)
analyzer = StaticAnalyzer()
result = analyzer.analyze_skill(skill_dir)
flags = [ap.flag for ap in result.anti_patterns]
assert "MISSING_TRIGGER" in flags