From e1a3bda5dbe632bfed4efbf8741526f457788904 Mon Sep 17 00:00:00 2001 From: kamalsrini <6233046+kamalsrini@users.noreply.github.com> Date: Mon, 15 Jun 2026 17:30:44 -0700 Subject: [PATCH] feat: add skill fixture test harness --- .github/workflows/test-skill-fixtures.yml | 20 +++ CONTRIBUTING.md | 6 + README.md | 6 + scripts/test_skill_fixtures.rb | 152 ++++++++++++++++++ tests/fixtures/README.md | 41 +++++ .../_example/static-evidence/manifest.yaml | 9 ++ .../_example/static-evidence/sample.txt | 4 + 7 files changed, 238 insertions(+) create mode 100644 .github/workflows/test-skill-fixtures.yml create mode 100644 scripts/test_skill_fixtures.rb create mode 100644 tests/fixtures/README.md create mode 100644 tests/fixtures/_example/static-evidence/manifest.yaml create mode 100644 tests/fixtures/_example/static-evidence/sample.txt diff --git a/.github/workflows/test-skill-fixtures.yml b/.github/workflows/test-skill-fixtures.yml new file mode 100644 index 00000000..1eeca67e --- /dev/null +++ b/.github/workflows/test-skill-fixtures.yml @@ -0,0 +1,20 @@ +name: Test Skill Fixtures + +on: + pull_request: + push: + branches: + - main + +permissions: + contents: read + +jobs: + test-skill-fixtures: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - name: Test skill fixtures + run: ruby scripts/test_skill_fixtures.rb diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 033e3034..6f2f9eec 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -152,6 +152,12 @@ and is enforced by CI. Run it locally before opening a PR: ruby scripts/validate_skill_schema.rb ``` +If your contribution includes skill test fixtures, also run: + +```bash +ruby scripts/test_skill_fixtures.rb +``` + --- ## Getting Started diff --git a/README.md b/README.md index dc29dfcb..86d49340 100644 --- a/README.md +++ b/README.md @@ -100,6 +100,12 @@ and role bundles locally with: ruby scripts/validate_skill_schema.rb ``` +Validate skill fixture manifests and expected evidence strings with: + +```bash +ruby scripts/test_skill_fixtures.rb +``` + ### Progressive disclosure (keep `SKILL.md` lean) Claude's skill guidance: when a `SKILL.md` would exceed ~500 lines, **don't inline everything** — split detail into sibling reference files in the same directory and link to them from `SKILL.md`. The agent loads a reference only when it needs it, so the entrypoint stays cheap to load. diff --git a/scripts/test_skill_fixtures.rb b/scripts/test_skill_fixtures.rb new file mode 100644 index 00000000..98871452 --- /dev/null +++ b/scripts/test_skill_fixtures.rb @@ -0,0 +1,152 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +require "yaml" + +ROOT = File.expand_path("..", __dir__) +FIXTURE_ROOT = File.join(ROOT, "tests", "fixtures") +KINDS = %w[vulnerable benign].freeze +REQUIRED_MANIFEST_FIELDS = %w[skill case_id kind target expected_findings].freeze +REQUIRED_FINDING_FIELDS = %w[id severity evidence_contains].freeze + +def rel(path) + path.delete_prefix("#{ROOT}#{File::SEPARATOR}") +end + +def load_manifest(path) + YAML.safe_load(File.read(path), permitted_classes: [], aliases: false) || {} +rescue Psych::SyntaxError => e + raise "invalid YAML: #{e.message}" +end + +def fixture_manifests + return [] unless Dir.exist?(FIXTURE_ROOT) + + Dir.glob(File.join(FIXTURE_ROOT, "*", "*", "manifest.yaml")).sort +end + +def validate_manifest_shape(manifest, manifest_path, errors) + unless manifest.is_a?(Hash) + errors << "#{rel(manifest_path)}: manifest must be a YAML object" + return + end + + REQUIRED_MANIFEST_FIELDS.each do |field| + errors << "#{rel(manifest_path)}: missing required field: #{field}" unless manifest.key?(field) + end + + errors << "#{rel(manifest_path)}: kind must be one of #{KINDS.join(', ')}" if manifest["kind"] && !KINDS.include?(manifest["kind"]) + + findings = manifest["expected_findings"] + unless findings.is_a?(Array) + errors << "#{rel(manifest_path)}: expected_findings must be an array" + return + end + + if manifest["kind"] == "benign" && !findings.empty? + errors << "#{rel(manifest_path)}: benign cases must use expected_findings: []" + end + + findings.each_with_index do |finding, index| + prefix = "#{rel(manifest_path)}: expected_findings[#{index}]" + unless finding.is_a?(Hash) + errors << "#{prefix} must be an object" + next + end + + REQUIRED_FINDING_FIELDS.each do |field| + errors << "#{prefix}: missing required field: #{field}" unless finding.key?(field) + end + + unless finding.key?("cwe") || finding.key?("framework") + errors << "#{prefix}: must include cwe or framework" + end + + evidence = finding["evidence_contains"] + next if evidence.nil? + + unless evidence.is_a?(String) && !evidence.empty? + errors << "#{prefix}: evidence_contains must be a non-empty string" + end + end +end + +def validate_case_paths(manifest, manifest_path, errors) + case_dir = File.dirname(manifest_path) + case_id = File.basename(case_dir) + skill_id = File.basename(File.dirname(case_dir)) + + errors << "#{rel(manifest_path)}: skill must match fixture directory '#{skill_id}'" if manifest["skill"] && manifest["skill"] != skill_id + errors << "#{rel(manifest_path)}: case_id must match fixture directory '#{case_id}'" if manifest["case_id"] && manifest["case_id"] != case_id + unless skill_id == "_example" || Dir.glob(File.join(ROOT, "skills", "*", skill_id, "SKILL.md")).any? + errors << "#{rel(manifest_path)}: skill fixture directory does not match an existing skill: #{skill_id}" + end + + target = manifest["target"] + unless target.is_a?(String) && !target.empty? + errors << "#{rel(manifest_path)}: target must be a non-empty relative path" + return nil + end + + if target.start_with?("/") || target.split(File::SEPARATOR).include?("..") + errors << "#{rel(manifest_path)}: target must stay inside the case directory" + return nil + end + + target_path = File.expand_path(target, case_dir) + unless target_path.start_with?("#{File.expand_path(case_dir)}#{File::SEPARATOR}") || target_path == File.expand_path(case_dir) + errors << "#{rel(manifest_path)}: target must stay inside the case directory" + return nil + end + + errors << "#{rel(manifest_path)}: target does not exist: #{target}" unless File.file?(target_path) + target_path +end + +def validate_evidence(manifest, manifest_path, target_path, errors) + return unless target_path && File.file?(target_path) + + findings = manifest["expected_findings"] + return unless findings.is_a?(Array) + + target_text = File.read(target_path) + findings.each do |finding| + next unless finding.is_a?(Hash) + + evidence = finding["evidence_contains"] + next unless evidence.is_a?(String) + + unless target_text.include?(evidence) + finding_id = finding["id"] || "(missing id)" + errors << "#{rel(manifest_path)}: finding #{finding_id} evidence_contains not found in #{rel(target_path)}" + end + end +end + +manifests = fixture_manifests + +if manifests.empty? + warn "FAIL: no skill fixture manifests found under tests/fixtures///manifest.yaml" + exit 1 +end + +errors = [] +manifests.each do |manifest_path| + begin + manifest = load_manifest(manifest_path) + validate_manifest_shape(manifest, manifest_path, errors) + target_path = validate_case_paths(manifest, manifest_path, errors) + validate_evidence(manifest, manifest_path, target_path, errors) + rescue StandardError => e + errors << "#{rel(manifest_path)}: #{e.message}" + end +end + +if errors.empty? + puts "OK: validated #{manifests.size} skill fixture manifest(s)." +else + puts "FAIL: skill fixture validation failed." + errors.each { |error| puts " - #{error}" } +end + +exit(errors.empty? ? 0 : 1) diff --git a/tests/fixtures/README.md b/tests/fixtures/README.md new file mode 100644 index 00000000..7bdb19f4 --- /dev/null +++ b/tests/fixtures/README.md @@ -0,0 +1,41 @@ +# Skill Fixture Tests + +Fixture cases live under: + +```text +tests/fixtures/// +``` + +Each case must include a `manifest.yaml`: + +```yaml +skill: secure-code-review +case_id: hardcoded-secret +kind: vulnerable +target: sample.cs +expected_findings: + - id: hardcoded-secret + severity: high + cwe: CWE-798 + evidence_contains: 'Password = "example"' +``` + +Required manifest fields: + +- `skill`: must match the `` directory. +- `case_id`: must match the `` directory. +- `kind`: `vulnerable` or `benign`. +- `target`: relative path to a file inside the case directory. +- `expected_findings`: array of expected structured findings. + +Each expected finding must include `id`, `severity`, `evidence_contains`, and +either `cwe` or `framework`. Benign cases must use `expected_findings: []`. + +Run the fixture harness locally with: + +```bash +ruby scripts/test_skill_fixtures.rb +``` + +The `_example` fixture is intentionally tiny. It only keeps the harness and CI +executable until real per-skill fixtures are added. diff --git a/tests/fixtures/_example/static-evidence/manifest.yaml b/tests/fixtures/_example/static-evidence/manifest.yaml new file mode 100644 index 00000000..672c22d6 --- /dev/null +++ b/tests/fixtures/_example/static-evidence/manifest.yaml @@ -0,0 +1,9 @@ +skill: _example +case_id: static-evidence +kind: vulnerable +target: sample.txt +expected_findings: + - id: example-hardcoded-secret + severity: medium + cwe: CWE-798 + evidence_contains: "password = \"not-a-real-secret\"" diff --git a/tests/fixtures/_example/static-evidence/sample.txt b/tests/fixtures/_example/static-evidence/sample.txt new file mode 100644 index 00000000..916bc03c --- /dev/null +++ b/tests/fixtures/_example/static-evidence/sample.txt @@ -0,0 +1,4 @@ +This tiny fixture keeps the static harness executable before real skill +fixtures land. + +password = "not-a-real-secret"