diff --git a/.github/scripts/qa-resolve-skills.sh b/.github/scripts/qa-resolve-skills.sh new file mode 100755 index 0000000..c836082 --- /dev/null +++ b/.github/scripts/qa-resolve-skills.sh @@ -0,0 +1,122 @@ +#!/usr/bin/env bash +# Resolve which skills a QA workflow should run, and write the JSON arrays to +# $GITHUB_OUTPUT as `skills` and `deleted_skills`. +# +# skills — runnable changed skills (eval these). +# deleted_skills — base skills removed at head (deletion/rename) → blocking manual review. +# +# Modes (exactly one): +# --skill NAME Explicit request (dispatch input, /qa-eval argument). A NAME with no +# skills/NAME/SKILL.md fails loudly: silently resolving to "nothing" +# has the same shape as "evaluated, found no work", which is the same +# shape as success. +# --pr NUMBER Skills changed anywhere in the PR (skills/ or testing/ paths), per the +# GitHub files API. Known ceiling: the endpoint returns at most 3,000 +# changed files per PR; PR-size expectations live in CONTRIBUTING.md. +# --all Every skill on disk (no deleted/unsupported filtering — the mutation +# battery's own context decides). +# +# "Skill" = a directory under skills/ containing SKILL.md — DERIVED from the tree, never +# a hardcoded list, so a brand-new skill in a community PR is in scope the moment it +# exists. The changed-path list is only ever CANDIDATES: testing/ also holds non-skill +# directories and loose files, so a path-derived name with no SKILL.md is dropped with a +# ::notice:: rather than passed through to fail deep inside the harness on a skill that +# does not exist. +# +# Used by qa-mutation-battery.yml from its own (trusted) checkout. qa-eval.yml fetches +# it from the DEFAULT branch at runtime: that job runs with credentials against PR-head +# content, and its sparse checkout deliberately excludes .github/ so no PR-provided +# executable ever runs there — consistent with issue_comment workflows, whose YAML itself +# is read from the default branch. qa-pr-screen.yml takes a single required skill input +# and does its own two-line existence check instead. +# +# Requires: a git checkout of the ref under test as CWD, jq; gh + GH_TOKEN for --pr. +set -euo pipefail + +SKILL="" +PR="" +ALL=0 +while [ $# -gt 0 ]; do + case "$1" in + --skill) SKILL="$2"; shift 2 ;; + --pr) PR="$2"; shift 2 ;; + --all) ALL=1; shift ;; + *) echo "::error::qa-resolve-skills.sh: unknown argument '$1'"; exit 2 ;; + esac +done +MODES=0 +[ -n "$SKILL" ] && MODES=$((MODES + 1)) +[ -n "$PR" ] && MODES=$((MODES + 1)) +[ "$ALL" -eq 1 ] && MODES=$((MODES + 1)) +if [ "$MODES" -ne 1 ]; then + echo "::error::qa-resolve-skills.sh: pass exactly one of --skill NAME, --pr NUMBER, --all" + exit 2 +fi + +ALL_SKILLS=$( + for d in skills/*/; do + [ -f "${d}SKILL.md" ] || continue + basename "$d" + done | sort -u | jq -R . | jq -sc . +) + +if [ -n "$SKILL" ]; then + CANDIDATES=$(jq -cn --arg s "$SKILL" '[$s]') + DELETED="[]" +elif [ -n "$PR" ]; then + # The `gh api` call stays on its own line so a failure fails the script CLOSED under + # set -e — not silently read as "no skill changes". Filename + per-file status arrive as + # TSV so we can see REMOVED paths under skills/ — those are skills that EXISTED at the + # base but may no longer exist at head (deletion/rename). A filename-only list misses + # them: the head-derived ALL_SKILLS no longer contains a deleted skill, so its dir would + # be dropped as "not a skill" and the PR would seed "no skill changes" (success). The + # separate `DELETED` extraction is the base-skill detection that closes that bypass. + FILES=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR}/files" --paginate \ + --jq '.[] | [.filename, .status, (.previous_filename // "")] | @tsv') + CANDIDATES=$(printf '%s' "$FILES" \ + | awk -F'\t' ' + $1 ~ /^(skills|testing)\// { + d=$1; sub(/^(skills|testing)\//,"",d); sub(/\/.*/,"",d); print d + } + $2=="renamed" && $3 ~ /^(skills|testing)\// { + d=$3; sub(/^(skills|testing)\//,"",d); sub(/\/.*/,"",d); print d + }' \ + | sort -u \ + | jq -R . | jq -sc .) + DELETED=$(printf '%s' "$FILES" \ + | awk -F'\t' ' + $2=="removed" && $1 ~ /^skills\// { + d=$1; sub(/^skills\//,"",d); sub(/\/.*/,"",d); print d + } + $2=="renamed" && $3 ~ /^skills\// { + d=$3; sub(/^skills\//,"",d); sub(/\/.*/,"",d); print d + }' \ + | sort -u \ + | jq -R . | jq -sc .) +else + CANDIDATES="$ALL_SKILLS" + DELETED="[]" +fi + +SKILLS=$(jq -cn --argjson c "$CANDIDATES" --argjson all "$ALL_SKILLS" '$c - ($c - $all)') +# Base skills that no longer exist at head (removed entirely from skills/). The gate cannot +# eval a skill it can no longer see, so these ride out as a blocking manual-review signal +# instead of silently resolving to "changed nothing". A rename produces one entry here (the +# old name) plus the new name in SKILLS — both are surfaced, neither disappears. +DELETED_SKILLS=$(jq -cn --argjson d "$DELETED" --argjson all "$ALL_SKILLS" '$d - $all') +DROPPED=$(jq -cn --argjson c "$CANDIDATES" --argjson all "$ALL_SKILLS" '$c - $all') +echo "skills=$SKILLS" >> "$GITHUB_OUTPUT" +echo "deleted_skills=$DELETED_SKILLS" >> "$GITHUB_OUTPUT" +echo "unsupported_skills=[]" >> "$GITHUB_OUTPUT" +echo "Will run: $SKILLS" +if [ "$DELETED_SKILLS" != "[]" ]; then + echo "::warning::Skills deleted/renamed in this PR (manual review required): $DELETED_SKILLS" +fi +# Say what was dropped. A silent skip and "nothing to run" look identical, and for a path +# that SHOULD have been a skill that is the wrong thing to be quiet about. +[ "$DROPPED" = "[]" ] || echo "::notice::Ignored non-skill paths: $DROPPED" + +if [ -n "$SKILL" ] && [ "$SKILLS" = "[]" ]; then + echo "::error::No skill named '$SKILL' under skills/. Available: $ALL_SKILLS" + exit 1 +fi diff --git a/.github/workflows/qa-eval-manual-clear.yml b/.github/workflows/qa-eval-manual-clear.yml new file mode 100644 index 0000000..6815654 --- /dev/null +++ b/.github/workflows/qa-eval-manual-clear.yml @@ -0,0 +1,98 @@ +name: QA Eval Manual Approval + +on: + workflow_dispatch: + inputs: + pr_number: + description: Pull request number + required: true + type: string + head_sha: + description: Exact pull request head SHA + required: true + type: string + reason: + description: Reason for manual approval + required: true + type: string + evidence_url: + description: HTTPS link to review evidence + required: true + type: string + +permissions: {} + +jobs: + approve: + runs-on: ubuntu-latest + environment: qa-eval-manual-review + permissions: + contents: read + pull-requests: read + checks: write + steps: + - name: Validate reviewer and exact PR head + id: validate + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ inputs.pr_number }} + EXPECTED_SHA: ${{ inputs.head_sha }} + EVIDENCE_URL: ${{ inputs.evidence_url }} + run: | + set -euo pipefail + case "$EVIDENCE_URL" in + https://*) ;; + *) echo "::error::evidence_url must use HTTPS"; exit 1 ;; + esac + case "$PR_NUMBER" in + *[!0-9]*|'') echo "::error::pr_number must be numeric"; exit 1 ;; + esac + if ! [[ "$EXPECTED_SHA" =~ ^[0-9a-f]{40}$ ]]; then + echo "::error::head_sha must be a full lowercase commit SHA" + exit 1 + fi + PERMISSION=$(gh api "repos/${{ github.repository }}/collaborators/${{ github.actor }}/permission" \ + --jq '.permission') + case "$PERMISSION" in + admin|maintain|write) ;; + *) echo "::error::${{ github.actor }} is not authorized to approve QA"; exit 1 ;; + esac + ACTUAL_SHA=$(gh api "repos/${{ github.repository }}/pulls/$PR_NUMBER" --jq '.head.sha') + if [ "$ACTUAL_SHA" != "$EXPECTED_SHA" ]; then + echo "::error::PR head is $ACTUAL_SHA, not $EXPECTED_SHA" + exit 1 + fi + CHECK_RUN_ID=$(gh api \ + "repos/${{ github.repository }}/commits/$EXPECTED_SHA/check-runs?check_name=qa-eval&filter=latest" \ + --jq '.check_runs[0] | select(.status == "completed" and .conclusion == "action_required") | .id // empty') + if [ -z "$CHECK_RUN_ID" ]; then + echo "::error::No action_required qa-eval check exists on $EXPECTED_SHA" + exit 1 + fi + echo "check_run_id=$CHECK_RUN_ID" >> "$GITHUB_OUTPUT" + + - name: Record manual approval + env: + GH_TOKEN: ${{ github.token }} + CHECK_RUN_ID: ${{ steps.validate.outputs.check_run_id }} + PR_NUMBER: ${{ inputs.pr_number }} + HEAD_SHA: ${{ inputs.head_sha }} + REASON: ${{ inputs.reason }} + EVIDENCE_URL: ${{ inputs.evidence_url }} + run: | + set -euo pipefail + { + echo "Manual QA approval recorded." + echo "" + echo "- PR: #$PR_NUMBER" + echo "- Head SHA: \`$HEAD_SHA\`" + echo "- Reviewer: @${{ github.actor }}" + echo "- Reason: $REASON" + echo "- Evidence: $EVIDENCE_URL" + echo "- Workflow run: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}" + } > "$RUNNER_TEMP/summary.md" + gh api -X PATCH "repos/${{ github.repository }}/check-runs/$CHECK_RUN_ID" \ + -f status=completed \ + -f conclusion=success \ + -f "output[title]=QA eval manually approved" \ + -F "output[summary]=@$RUNNER_TEMP/summary.md" > /dev/null diff --git a/.github/workflows/qa-eval-status.yml b/.github/workflows/qa-eval-status.yml new file mode 100644 index 0000000..f0d9fe9 --- /dev/null +++ b/.github/workflows/qa-eval-status.yml @@ -0,0 +1,130 @@ +# qa-eval required-check seeder. +# +# Manages the DEFAULT state of the `qa-eval` check run so a PR can't merge until a QA +# eval has run on the current head: +# - PR changes no evaluatable skill content -> completed/success ("not required") +# - PR changes skills//** or testing//** for a real skill -> queued +# ("run /qa-eval") +# - PR deletes or renames a skill (removed skills//** where the skill no longer +# exists at head) -> completed/action_required ("manual review"): a deleted skill +# cannot be auto-evaluated, so it must NOT collapse into the "no skill changes" success. +# A `queued` check run renders neutral (not a red failure) so it isn't confused with a +# broken build. `action_required` BLOCKS the required check until a maintainer resolves it. +# +# "Evaluatable skill content" is decided by .github/scripts/qa-resolve-skills.sh — the +# SAME resolver qa-eval.yml runs — so this seeder and the gate cannot disagree about what +# requires evaluation. An earlier grep here counted only skills/** while the resolver also +# counts testing//**: a tests/fixtures-only PR got an instant success and merged +# without the suite it modified ever running. +# +# The gate is a CHECK RUN; qa-eval.yml carries the eval report inline on it. Check runs +# are per-SHA by construction, which makes the freshness problem disappear: a pass on an +# old commit never satisfies a new head, because the new head simply has no completed +# check run. +# +# NOTE: this creates the check run, but does not by itself make merging conditional on +# it -- that requires "qa-eval" to be added under required checks in this repo's ruleset, +# a separate, repo-admin action. +# +# NOTE (forks): on fork PRs the built-in GITHUB_TOKEN is read-only, so check-run creation +# is denied. The job-level `if:` skips fork PRs; the check then stays uncreated -> shows +# "Expected" -> blocked by absence IF the check is required. A maintainer comments +# `/qa-eval` on the fork PR and qa-eval.yml (which runs in the base-repo context with a +# read-write token) creates the check run itself. So forks are covered end-to-end without +# this seeder. +# +# KILL SWITCH: the whole gate is inert until the repo variable QA_EVAL_ENABLED is set to +# 'true'. Flip it only after 10gen/agent-skills-evals#18 (the harness) has merged and a +# dispatch smoke test of qa-eval.yml has gone green. +name: QA Eval Status + +on: + pull_request: + types: [opened, synchronize, reopened] + +permissions: + contents: read # sparse-checkout the PR head (skills/ + testing/) for the resolver + pull-requests: read + checks: write + +# One seeder run per PR head SHA. cancel-in-progress: true is safe here because the +# trigger is `pull_request` (not issue_comment): only a new push to the same PR head +# supersedes, so an unrelated event never kills an in-flight seed. A stale seed for an +# old SHA is harmless — qa-eval.yml posts onto the SHA it actually evaluated, and a +# superseding push re-seeds the new head. +concurrency: + group: qa-eval-status-${{ github.event.pull_request.head.sha }} + cancel-in-progress: true + +jobs: + seed: + if: ${{ github.event.pull_request.head.repo.fork == false && vars.QA_EVAL_ENABLED == 'true' }} + runs-on: ubuntu-latest + steps: + - name: Check out the PR head (data only, skills/ + testing/) + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 + with: + ref: ${{ github.event.pull_request.head.sha }} + persist-credentials: false + sparse-checkout: | + skills + testing + + # From the DEFAULT branch, not the checkout above: a pull_request run reads its YAML + # from the merge ref, so a PR could otherwise edit the resolver to seed its own + # "success". Same fetch qa-eval.yml uses. + - name: Fetch the skill-resolution script (default branch) + env: + GH_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + gh api "repos/${{ github.repository }}/contents/.github/scripts/qa-resolve-skills.sh" \ + -H "Accept: application/vnd.github.raw" > "$RUNNER_TEMP/qa-resolve-skills.sh" + chmod +x "$RUNNER_TEMP/qa-resolve-skills.sh" + + # A resolver failure (its gh api call runs under set -e) fails this job with NO check + # run seeded: a required check then stays "Expected" and blocks merge. Fail-closed, + # never silently seeding success on an API error. + - name: Resolve skills changed in this PR + id: list + env: + GH_TOKEN: ${{ github.token }} + run: | + "$RUNNER_TEMP/qa-resolve-skills.sh" --pr "${{ github.event.pull_request.number }}" + + - name: Seed qa-eval check run + env: + GH_TOKEN: ${{ github.token }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + SKILLS: ${{ steps.list.outputs.skills }} + DELETED: ${{ steps.list.outputs.deleted_skills }} + run: | + set -euo pipefail + DELETED="${DELETED:-[]}" + # Fail-closed first: a deleted/renamed skill can't be auto-evaluated, and seeding + # "no skill changes" success would let the removal bypass the required QA gate. + # action_required blocks merge until a maintainer manually reviews the removal. + if [ "$DELETED" != "[]" ]; then + gh api -X POST "repos/${{ github.repository }}/check-runs" \ + -f name=qa-eval \ + -f head_sha="$HEAD_SHA" \ + -f status=completed \ + -f conclusion=action_required \ + -f "output[title]=Skill deletion/rename requires manual review" \ + -f "output[summary]=This PR removes \`skills//\` for: $DELETED. Removed skills cannot be auto-evaluated, so the \`qa-eval\` gate is blocked pending a maintainer's manual review." + elif [ "$SKILLS" = "[]" ]; then + gh api -X POST "repos/${{ github.repository }}/check-runs" \ + -f name=qa-eval \ + -f head_sha="$HEAD_SHA" \ + -f status=completed \ + -f conclusion=success \ + -f "output[title]=No skill changes" \ + -f "output[summary]=This PR changes no \`skills//\` or \`testing//\` content for any skill; the QA eval is not required." + else + gh api -X POST "repos/${{ github.repository }}/check-runs" \ + -f name=qa-eval \ + -f head_sha="$HEAD_SHA" \ + -f status=queued \ + -f "output[title]=QA eval required before merge" \ + -f "output[summary]=This PR changes skill or eval content ($SKILLS). A maintainer runs \`/qa-eval\` in a PR comment to produce the verdict (see .github/workflows/qa-eval.yml)." + fi diff --git a/.github/workflows/qa-eval.yml b/.github/workflows/qa-eval.yml new file mode 100644 index 0000000..8ee0345 --- /dev/null +++ b/.github/workflows/qa-eval.yml @@ -0,0 +1,504 @@ +# qa-eval — capability gate for Agent Skills. +# +# Runs the Inspect-AI QA harness (10gen/agent-skills-evals, inspect/) against the skill +# content changed in a PR and reports the verdict as the `qa-eval` check run (seeded by +# qa-eval-status.yml). Triggered manually by a maintainer comment so the Grove (LLM) +# spend is tied to deliberate runs, not every push. +# +# Trigger: comment `/qa-eval` (optionally `/qa-eval `) on a PR. OWNER/MEMBER only. +# workflow_dispatch with a pr_number is the smoke-test path. +# Enforce: the `main` ruleset requires the `qa-eval` check; qa-eval-status.yml seeds it. +# The check stands for FULL coverage: an explicit-skill run on a multi-skill +# PR concludes failure ("partial coverage") naming the skills still to eval. +# +# The verdict rides on the check run itself — report inline in output.summary, per-skill +# JSON as artifacts — with no committed report file and no PR-comment duplicate: +# - freshness is inherent: the workflow posts onto the very SHA it evaluated; +# - fork PRs are covered: check runs are created in the base repo's context, so there +# is nothing to push to a fork branch; +# - git history stays free of machine-generated commits; +# - and because nothing posts to the PR conversation, this job's token carries no write +# scope but checks: write (see the security note below). +# +# Security — `issue_comment` is a privileged trigger, and this single job both checks +# out PR-provided content and holds the Grove API key + the agent-skills-evals App PEM. +# CodeQL flags that combination as "untrusted checkout in a privileged context." We +# accept the alert on these grounds (mirrors skill-gate.yml PR #51, approved under +# ENTSEC-5465): +# - The checkout is DATA-ONLY: SHA-pinned to the PR head, persist-credentials false, +# sparse skills/ + testing/ tree (excludes .github/, etc.). Nothing from the PR is +# built. +# - The GITHUB_TOKEN's only write scope is checks: write (driving the check run) — no +# contents: write and no pull-requests: write, so exfiltration can neither tamper +# with the repo nor post to the PR. The Grove key only egresses to Grove and is +# gated behind the qa-eval environment's required reviewers. +# This alert re-fires on pushes that touch this checkout and should be dismissed in the +# code-scanning Security tab with the justification above. +# +# HONEST CAVEAT: unlike skill-gate (which reads markdown as DATA with a pinned binary), +# this job EXECUTES the Inspect-AI harness against untrusted eval-case YAML with the +# Grove key in its env. The OWNER/MEMBER-only trigger is the actual control for that; +# do not widen it. A two-job split would not change this — the harness still runs +# untrusted content with the key present; the split only keeps the git checkout off the +# credential-bearing runner, and persist-credentials: false + sparse checkout already +# limit that surface. (The prior split was collapsed at review request; see PR #51.) +# +# PUBLIC-CONTENT RULE: this is a public repo — job logs, artifacts, and check-run output +# are all world-visible. The check-run summary is built only from fields we generate +# ourselves (skill names, verdicts, integer counts, URLs) — never model output or +# transcript text. If that ever changes, the content must go through +# agent-skills-evals/inspect/analysis/sanitize.py (public_safe_run) first, the same +# allowlisting screen_report.py uses. +# +# Prereqs (security review: ENTSEC-5465): +# - repo variable QA_EVAL_ENABLED = 'true' (kill switch; also arms qa-eval-status.yml) +# - GitHub environment `qa-eval` with secret GROVE_API_KEY (the Grove model-call key) +# - repo variable GROVE_BASE_URL (Grove base URL; non-sensitive) +# - repo secret AGENT_SKILLS_EVALS_APP_ID (App ID; non-sensitive) +# - repo secret AGENT_SKILLS_EVALS_APP_PRIVATE_KEY (App PEM; used to checkout +# 10gen/agent-skills-evals — repo-level so both the qa-eval and qa-eval-publish +# environments reach it) +# - 10gen/agent-skills-evals#18 merged (provides harness.py + scripts/ci_gate.py) +# +# INCONCLUSIVE (instrument fault: Grove 401, MCP registration failure, harness crash) +# maps to check-run conclusion `failure`, NOT `neutral`: GitHub rulesets treat `neutral` +# (like `skipped`) as SATISFYING a required check, so an inconclusive run would wave the +# PR through unmeasured. The failure title ("re-run needed") is what distinguishes infra +# from a real eval failure. Accepted cost: a Grove outage blocks the gate until fixed. +name: QA Eval + +on: + issue_comment: + types: [created] + workflow_dispatch: + inputs: + pr_number: + description: "PR number to eval" + required: true + type: string + skill: + description: "Skill to eval (blank = all skills changed in the PR)" + required: false + type: string + +# Built-in GITHUB_TOKEN only (ephemeral, repo-scoped). Nothing granted at the top level; +# permissions are declared per job (least privilege). +permissions: {} + +# Serialize per PR, but do NOT cancel an in-flight run: a run costs real model spend and +# up to ~an hour, and a cancelled job leaves the check run stuck in_progress. A second +# /qa-eval comment simply queues behind the first. +concurrency: + group: qa-eval-${{ github.event.issue.number || github.event.inputs.pr_number }} + cancel-in-progress: false + +env: + # Kept in lockstep with qa-pr-screen.yml and harness.py's default. + PANEL_MODEL: anthropic/claude-sonnet-4-6 + # A CONCURRENCY cap, not a case count: each sample of a functional task spins its own + # agent + mongo container pair. Matches qa-pr-screen.yml's sizing for 2-vCPU runners. + MAX_SAMPLES: "2" + +jobs: + eval: + if: >- + vars.QA_EVAL_ENABLED == 'true' && + ( + github.event_name == 'workflow_dispatch' || + ( + github.event.issue.pull_request && + ( + github.event.comment.body == '/qa-eval' || + startsWith(github.event.comment.body, '/qa-eval ') + ) && + contains(fromJSON('["OWNER","MEMBER"]'), github.event.comment.author_association) + ) + ) + runs-on: ubuntu-latest + environment: qa-eval + permissions: + contents: read + pull-requests: read # resolve the PR head/base + list its changed files + checks: write # drive the `qa-eval` check run lifecycle + timeout-minutes: 90 + env: + # Grove key from the qa-eval environment secret. Base URL is non-sensitive and + # stays a repo var. + GROVE_API_KEY: ${{ secrets.GROVE_API_KEY }} + GROVE_BASE_URL: ${{ vars.GROVE_BASE_URL }} + steps: + - name: Resolve PR head/base + id: pr + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event_name == 'workflow_dispatch' && inputs.pr_number || github.event.issue.number }} + run: | + set -euo pipefail + # issue_comment runs in the default-branch context, so github.sha is main's + # tip — NOT the PR head. Resolve the real head sha to eval it and to attach + # the check run where the ruleset will look for it. + read -r head_sha base_sha <<<"$(gh api "repos/${{ github.repository }}/pulls/$PR_NUMBER" \ + --jq '.head.sha + " " + .base.sha')" + echo "head_sha=$head_sha" >> "$GITHUB_OUTPUT" + echo "base_sha=$base_sha" >> "$GITHUB_OUTPUT" + echo "pr_number=$PR_NUMBER" >> "$GITHUB_OUTPUT" + + - name: Check out skill + eval content (data only) + # DATA-ONLY untrusted checkout: SHA-pinned to the PR head, no persisted token, + # sparse skills/ + testing/ tree (excludes .github/, etc.). Nothing here is + # built — it is read by the harness below. See the header comment for why the + # CodeQL "untrusted checkout" alert is accepted for this checkout. + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 + with: + ref: ${{ steps.pr.outputs.head_sha }} + persist-credentials: false + sparse-checkout: | + skills + testing + + # The skill-resolution script is fetched from the DEFAULT branch, not read from the + # checkout above: the sparse tree deliberately excludes .github/ so that no + # PR-provided executable ever runs in this credential-bearing job. This is + # consistent with issue_comment workflows generally, whose YAML itself is read from + # the default branch. + - name: Fetch the skill-resolution script (default branch) + env: + GH_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + gh api "repos/${{ github.repository }}/contents/.github/scripts/qa-resolve-skills.sh" \ + -H "Accept: application/vnd.github.raw" > "$RUNNER_TEMP/qa-resolve-skills.sh" + chmod +x "$RUNNER_TEMP/qa-resolve-skills.sh" + + - name: Determine which skill(s) to eval + id: list + env: + SKILL_ARG: ${{ github.event_name == 'workflow_dispatch' && inputs.skill || '' }} + COMMENT_BODY: ${{ github.event.comment.body }} + PR_NUMBER: ${{ steps.pr.outputs.pr_number }} + GH_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + # Optional explicit skill: `/qa-eval ` or the dispatch input. First + # token of the comment must be exactly `/qa-eval`. The job `if` already requires + # an exact `/qa-eval` or `/qa-eval ` prefix, and this strict parse is the + # belt-and-braces inside the job for issue_comment runs. + REQUESTED="$SKILL_ARG" + if [ -z "$REQUESTED" ] && [ "${{ github.event_name }}" = "issue_comment" ]; then + FIRST_LINE=$(printf '%s' "$COMMENT_BODY" | head -1) + CMD=$(printf '%s' "$FIRST_LINE" | awk '{print $1}') + if [ "$CMD" != "/qa-eval" ]; then + echo "::notice::Comment does not start with the exact /qa-eval command; skipping." + echo "skills=[]" >> "$GITHUB_OUTPUT" + exit 0 + fi + REQUESTED=$(printf '%s' "$FIRST_LINE" | awk '{print $2}') + fi + + if [ -n "$REQUESTED" ]; then + "$RUNNER_TEMP/qa-resolve-skills.sh" --skill "$REQUESTED" + else + "$RUNNER_TEMP/qa-resolve-skills.sh" --pr "$PR_NUMBER" + fi + + # Nothing below runs when there are no skills to eval (the seeder's check-run state + # is left to stand). The `if: always()` finalize still runs only when there WAS work, + # so a no-op /qa-eval never posts a spurious verdict. + # + # There is no credential preflight here: missing model credentials fail loud in + # harness.py's own preflight (which names the missing vars), one definition for + # every caller, instead of a per-workflow shell guard. + + - name: Start the qa-eval check run (in_progress) + if: steps.list.outputs.skills != '[]' + env: + GH_TOKEN: ${{ github.token }} + HEAD_SHA: ${{ steps.pr.outputs.head_sha }} + run: | + set -euo pipefail + # The seeder normally created this check run (state queued). It may be absent — + # e.g. a fork PR, where the seeder skips but a member-triggered /qa-eval still + # runs here in the base-repo context (which is exactly how forks get covered). + CHECK_RUN_ID=$(gh api \ + "repos/${{ github.repository }}/commits/$HEAD_SHA/check-runs?check_name=qa-eval&filter=latest" \ + --jq '.check_runs[0].id // empty') + if [ -z "$CHECK_RUN_ID" ]; then + CHECK_RUN_ID=$(gh api -X POST "repos/${{ github.repository }}/check-runs" \ + -f name=qa-eval -f head_sha="$HEAD_SHA" -f status=queued --jq '.id') + fi + gh api -X PATCH "repos/${{ github.repository }}/check-runs/$CHECK_RUN_ID" \ + -f status=in_progress \ + -f "output[title]=QA eval running" \ + -f "output[summary]=Evaluating changed skills against the QA harness…" \ + > /dev/null + echo "CHECK_RUN_ID=$CHECK_RUN_ID" >> "$GITHUB_ENV" + + - name: Generate a token for the agent-skills-evals GitHub App + if: steps.list.outputs.skills != '[]' + id: app-token + uses: actions/create-github-app-token@fee1f7d63c2ff003460e3d139729b119787bc349 # v2.2.2 + with: + app-id: ${{ secrets.AGENT_SKILLS_EVALS_APP_ID }} + private-key: ${{ secrets.AGENT_SKILLS_EVALS_APP_PRIVATE_KEY }} + owner: 10gen + repositories: agent-skills-evals + + - name: Checkout agent-skills-evals (private, sibling checkout) + if: steps.list.outputs.skills != '[]' + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 + with: + repository: 10gen/agent-skills-evals + token: ${{ steps.app-token.outputs.token }} + path: agent-skills-evals + ref: ${{ vars.QA_EVALS_REF || 'main' }} + + - uses: astral-sh/setup-uv@38f3f104447c67c051c4a08e39b64a148898af3a # v4 + if: steps.list.outputs.skills != '[]' + with: + python-version: "3.12" + + - name: Resolve private runner capabilities + id: capabilities + if: steps.list.outputs.skills != '[]' + env: + REQUESTED_SKILLS: ${{ steps.list.outputs.skills }} + run: | + set -euo pipefail + cd agent-skills-evals/inspect + uv sync --quiet + RESULT=$(uv run python execution_profiles.py \ + --platform github --skills "$REQUESTED_SKILLS") + echo "runnable=$(jq -c '.runnable' <<<"$RESULT")" >> "$GITHUB_OUTPUT" + echo "unsupported=$(jq -c '.unsupported' <<<"$RESULT")" >> "$GITHUB_OUTPUT" + + - name: Run harness + gate per skill + id: scan + if: steps.capabilities.outputs.runnable != '[]' + env: + SKILLS: ${{ steps.capabilities.outputs.runnable }} + HEAD_SHA: ${{ steps.pr.outputs.head_sha }} + BASE_SHA: ${{ steps.pr.outputs.base_sha }} + SKILL_BASE_DIR: ${{ github.workspace }}/skills + QA_TESTING_DIR: ${{ github.workspace }}/testing + SEED_BASE_DIR: ${{ github.workspace }}/testing + run: | + set -euo pipefail + mkdir -p "$RUNNER_TEMP/reports" + cd agent-skills-evals/inspect + for SKILL in $(echo "$SKILLS" | jq -r '.[]'); do + echo "== QA eval: $SKILL (model $PANEL_MODEL, k=1) ==" + + set +e + uv run python harness.py --skill "$SKILL" --model "$PANEL_MODEL" \ + --platform github --epochs 1 --max-samples "$MAX_SAMPLES" + HARNESS_RC=$? + set -e + + # Only gate a log when the harness itself completed: a crashed harness may + # leave no log (or a partial one), and "no measurement" is INCONCLUSIVE, not + # FAIL. ci_gate reads the NEWEST log in logs/, which — running sequentially — + # is this skill's run. + if [ "$HARNESS_RC" -eq 0 ]; then + set +e + uv run python ../scripts/ci_gate.py logs --mode strict + GATE_RC=$? + set -e + else + GATE_RC=2 + echo "harness exited $HARNESS_RC for $SKILL — treating as INCONCLUSIVE" + fi + + case "$GATE_RC" in + 0) verdict=pass ;; + 1) verdict=fail ;; + *) verdict=inconclusive ;; + esac + + jq -cn \ + --arg skill "$SKILL" \ + --arg headSha "$HEAD_SHA" \ + --arg baseSha "$BASE_SHA" \ + --arg model "$PANEL_MODEL" \ + --arg verdict "$verdict" \ + --arg runAt="$(date -u +%Y-%m-%dT%H:%M:%SZ)" \ + '{skillName: $skill, headSha: $headSha, baseSha: $baseSha, model: $model, + verdict: $verdict, runAt: $runAt}' \ + > "$RUNNER_TEMP/reports/qa-eval-report-$SKILL.json" + done + + - name: Upload eval reports + if: always() && steps.list.outputs.skills != '[]' + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: qa-eval-report-${{ steps.pr.outputs.head_sha }} + path: ${{ runner.temp }}/reports + if-no-files-found: warn + + - name: Finalize the qa-eval check run + if: always() && steps.list.outputs.skills != '[]' + env: + GH_TOKEN: ${{ github.token }} + HEAD_SHA: ${{ steps.pr.outputs.head_sha }} + PR_NUMBER: ${{ steps.pr.outputs.pr_number }} + RUN_URL: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }} + UNSUPPORTED: ${{ steps.capabilities.outputs.unsupported }} + DELETED: ${{ steps.list.outputs.deleted_skills }} + run: | + set -euo pipefail + + # Normalize the resolver's manual-review sets to a literal JSON list so a missing + # output is never read as a deletion/unsupported signal that would falsely block. + UNSUPPORTED="${UNSUPPORTED:-[]}" + DELETED="${DELETED:-[]}" + + # The verdict table is derived from the per-skill report files the run step + # wrote — not assembled inline. No reports at all (the harness crashed before + # writing any, or the job was cancelled mid-run) is INCONCLUSIVE, and an + # unmeasured PR must fail the gate, not pass it: `neutral` would satisfy a + # ruleset's required check, so this concludes failure like any instrument fault. + shopt -s nullglob + REPORTS=("$RUNNER_TEMP"/reports/qa-eval-report-*.json) + + # COVERAGE: an explicit `/qa-eval ` (or dispatch input) evaluates a subset. + # The required check must stand for "every skill this PR changes was evaluated", + # so re-resolve the PR's changed skills and fail on under-coverage — otherwise a + # one-skill eval on a three-skill PR would green the gate. (Resolver writes to + # $GITHUB_OUTPUT; point it at a scratch file to reuse it mid-script. If the PR's + # file list changed mid-run, the new push re-seeds its own head, so a stale + # comparison here only ever makes the OLD head's check stricter.) + GITHUB_OUTPUT="$RUNNER_TEMP/coverage.out" \ + "$RUNNER_TEMP/qa-resolve-skills.sh" --pr "$PR_NUMBER" > /dev/null 2>&1 \ + && EXPECTED=$(sed -n 's/^skills=//p' "$RUNNER_TEMP/coverage.out" | jq -Sc 'sort') \ + || EXPECTED="" + EVALUATED=$( + if [ ${#REPORTS[@]} -gt 0 ]; then + # Real report files: slurp and dedupe. The `|| echo '[]'` fallback keeps a + # jq failure from wedging the check run. + jq -Scs '[.[].skillName] | unique' "${REPORTS[@]}" 2>/dev/null || echo '[]' + else + # No report files at all: short-circuit instead of calling jq with an empty + # argument list — a bare `jq -s` reads stdin, whose behavior depends on the + # runner and can hang a terminal session. + echo '[]' + fi + ) + COVERED=$(jq -cn --argjson e "$EVALUATED" --argjson u "$UNSUPPORTED" '$e + $u | unique | sort') + if [ -z "$EXPECTED" ]; then + # The coverage resolver itself failed (API outage). Still conclude — a wedged + # in_progress check blocks merge indefinitely — but as failure, not success. + INDEX="" + MISSING="" + CONCLUSION=failure + TITLE="QA eval could not verify coverage — re-run needed" + elif [ "$COVERED" != "$EXPECTED" ]; then + if [ ${#REPORTS[@]} -gt 0 ]; then + INDEX=$(jq -s '[.[] | {skill: .skillName, verdict}]' "${REPORTS[@]}") + else + INDEX="" + fi + MISSING=$(jq -cn --argjson e "$EXPECTED" --argjson g "$COVERED" '$e - $g | join(", ")') + CONCLUSION=failure + TITLE="QA eval incomplete — partial coverage" + elif [ ${#REPORTS[@]} -gt 0 ]; then + INDEX=$(jq -s '[.[] | {skill: .skillName, verdict}]' "${REPORTS[@]}") + FAILS=$(jq '[.[] | select(.verdict == "fail")] | length' <<<"$INDEX") + INCONCLUSIVE=$(jq '[.[] | select(.verdict == "inconclusive")] | length' <<<"$INDEX") + if [ "$FAILS" -gt 0 ]; then + CONCLUSION=failure + TITLE="QA eval failed" + elif [ "$INCONCLUSIVE" -gt 0 ]; then + # INCONCLUSIVE = instrument fault (Grove 401, MCP registration failure, + # harness crash), not a verdict on the PR. Conclusion is `failure`, not + # `neutral`: rulesets treat neutral as satisfying a required check, so an + # instrument fault would otherwise merge the PR through unmeasured. The + # title carries "re-run needed" so it isn't read as a verdict on the change. + CONCLUSION=failure + TITLE="QA eval inconclusive — re-run needed" + elif [ "$UNSUPPORTED" != "[]" ]; then + INDEX="" + CONCLUSION=action_required + TITLE="QA eval requires manual review" + else + CONCLUSION=success + TITLE="QA eval passed" + fi + elif [ "$UNSUPPORTED" != "[]" ]; then + INDEX="" + CONCLUSION=action_required + TITLE="QA eval requires manual review" + else + INDEX="" + CONCLUSION=failure + TITLE="QA eval did not produce results" + fi + + # Fail-closed for skills the eval did not (and cannot) measure. Even with every + # runnable skill passing, the gate must not go green while an unsupported or + # deleted skill's change goes unevaluated and unlooked-at. action_required blocks + # merge (like failure), but carries the "manual review" title instead of being + # read as a verdict on the change. + if [ "$UNSUPPORTED" != "[]" ] || [ "$DELETED" != "[]" ]; then + if [ "$CONCLUSION" != "failure" ]; then + CONCLUSION=action_required + TITLE="QA eval passed — manual review required" + fi + MANUAL_NOTES="" + [ "$DELETED" = "[]" ] || MANUAL_NOTES="${MANUAL_NOTES:+$MANUAL_NOTES }These skills were deleted/renamed in this PR and must be reviewed manually: $DELETED." + [ "$UNSUPPORTED" = "[]" ] || MANUAL_NOTES="${MANUAL_NOTES:+$MANUAL_NOTES }The pinned private harness has no GitHub runner profile for: $UNSUPPORTED." + fi + + # Summary is built ONLY from self-generated fields (skill names, verdicts, + # URLs) — no model output. See the PUBLIC-CONTENT RULE in the header. + { + echo "| Skill | Verdict |" + echo "|-------|---------|" + if [ -n "$INDEX" ]; then + jq -r '.[] | "| \(.skill) | \(.verdict) |"' <<<"$INDEX" + else + echo "| (all) | no results — see run log |" + fi + if [ -n "${MISSING:-}" ]; then + echo "" + echo "**Not evaluated:** $MISSING. Re-run \`/qa-eval\` with no skill argument to cover every changed skill." + fi + if [ -n "${MANUAL_NOTES:-}" ]; then + echo "" + echo "**Manual review required:** $MANUAL_NOTES" + fi + echo "" + echo "Model: \`$PANEL_MODEL\`. Full per-skill JSON reports: artifact" + echo "\`qa-eval-report-*\` on [the run page]($RUN_URL)." + } > "$RUNNER_TEMP/summary.md" + + # CHECK_RUN_ID is normally in the env from the start step. If the job was + # cancelled/timed out before that step wrote GITHUB_ENV (or the workflow was + # edited between runs), fall back to the same name lookup the start step does — + # or create the run outright. A wedged run must never leave the required check + # stuck in_progress, where it would block merge indefinitely. + CHECK_RUN_ID="${CHECK_RUN_ID:-}" + if [ -z "$CHECK_RUN_ID" ]; then + CHECK_RUN_ID=$(gh api \ + "repos/${{ github.repository }}/commits/$HEAD_SHA/check-runs?check_name=qa-eval&filter=latest" \ + --jq '.check_runs[0].id // empty') + fi + if [ -n "$CHECK_RUN_ID" ]; then + gh api -X PATCH \ + "repos/${{ github.repository }}/check-runs/$CHECK_RUN_ID" \ + -f status=completed \ + -f conclusion="$CONCLUSION" \ + -f "output[title]=$TITLE" \ + -F "output[summary]=@$RUNNER_TEMP/summary.md" > /dev/null + else + gh api -X POST "repos/${{ github.repository }}/check-runs" \ + -f name=qa-eval \ + -f head_sha="$HEAD_SHA" \ + -f status=completed \ + -f conclusion="$CONCLUSION" \ + -f "output[title]=$TITLE" \ + -F "output[summary]=@$RUNNER_TEMP/summary.md" > /dev/null + fi + + # The workflow's own exit code reflects the verdict so the run page and any + # `gh run watch` caller read it correctly. (The check run is the merge gate; + # this job's status is for operators.) + [ "$CONCLUSION" = "success" ] diff --git a/.github/workflows/qa-mutation-battery.yml b/.github/workflows/qa-mutation-battery.yml new file mode 100644 index 0000000..c9da7e4 --- /dev/null +++ b/.github/workflows/qa-mutation-battery.yml @@ -0,0 +1,249 @@ +name: QA Mutation Battery + +# Deliberately manual (`workflow_dispatch` only) until the triggers below are uncommented. +# +# What this does: for a skill, materializes each mutant in its mutation config +# (testing//evals/mutation.yaml), runs the QA harness against baseline + every +# mutant, and commits the resulting sensitivity report to +# agent-skills-evals/baselines/-mutation.json. Advisory only — never blocks a PR. +# It keeps the "what can this suite actually detect?" measurement fresh as skills change. +# +# Before uncommenting `push`/`schedule`: +# 1. The GitHub `qa-eval` environment must be provisioned (prereqs in qa-eval.yml's +# header). The Grove key is a `qa-eval` environment secret. +# 2. AGENT_SKILLS_EVALS_APP_ID + AGENT_SKILLS_EVALS_APP_PRIVATE_KEY are repo secrets. +# The App's existing Contents access on agent-skills-evals covers the checkout + +# baseline commit; no App changes are needed. + +on: + # push: + # branches: [main] + # paths: + # - "skills/*/SKILL.md" + # - "skills/*/references/**" + # - "testing/*/evals/mutation.yaml" + # schedule: + # - cron: "0 6 1 */3 *" # quarterly safety-net census, all skills + workflow_dispatch: + inputs: + skill: + description: "Skill to run the battery for (blank = every skill under skills/)" + required: false + type: string + epochs: + description: >- + Repeats per sample. Default 5: below that, run-to-run noise sits above the + battery's detection threshold and the report is suppressed as unmeasurable — + a lower-k run spends the model calls and learns nothing. + required: false + default: "5" + type: string + +# Serialize ALL batteries, not just per skill: keying the group on the skill input lets an +# all-skills run and a targeted run hold different locks while both publish the same +# baselines/-mutation.json. Batteries run for hours; a second dispatch queueing +# behind the first costs nothing. +concurrency: + group: qa-mutation-battery + cancel-in-progress: false + +permissions: + contents: read + +jobs: + detect: + runs-on: ubuntu-latest + outputs: + skills: ${{ steps.list.outputs.skills }} + steps: + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 + + - name: Determine which skill(s) to run + id: list + env: + SKILL_INPUT: ${{ github.event.inputs.skill }} + run: | + set -euo pipefail + if [ -n "$SKILL_INPUT" ]; then + .github/scripts/qa-resolve-skills.sh --skill "$SKILL_INPUT" + else + .github/scripts/qa-resolve-skills.sh --all + fi + + battery: + needs: detect + if: needs.detect.outputs.skills != '[]' + strategy: + fail-fast: false # one skill's battery failing must not cancel the others + max-parallel: 2 # bounded model-call spend and metadata-store contention + matrix: + skill: ${{ fromJSON(needs.detect.outputs.skills) }} + runs-on: ubuntu-latest + # Holds GROVE_API_KEY as an environment secret (gated behind required reviewers). + # Base URL is a non-sensitive repo var. + environment: qa-eval + permissions: + contents: read + # ~11 mutants plus a baseline, each at --epochs 5, is long but bounded. Without an + # explicit cap a wedged harness burns the 6h default before anyone notices. + timeout-minutes: 300 + services: + # job-local metadata store for qa_runs / mutation_report.py — ephemeral, no secret, + # unrelated to the functional-slice sandbox Mongo the harness seeds per sample. + mongo: + image: mongo:7 + ports: + - 27017:27017 + options: >- + --health-cmd "mongosh --eval 'db.runCommand({ping:1})'" + --health-interval 5s --health-timeout 5s --health-retries 10 + env: + MONGODB_URI: mongodb://localhost:27017 + MONGODB_DB: qa_mutation_battery + EPOCHS: ${{ github.event.inputs.epochs || '5' }} + # A CONCURRENCY cap, not a case count: each sample of a functional task spins its own + # agent + mongo container pair. 4 was sized for the k8s sandbox; on a 2-vCPU/7GB + # ubuntu-latest runner that is 8 containers, and samples OOM-killed into INVALID are + # samples the harness correctly refuses to score — a wasted run, not a cheap one. + MAX_SAMPLES: "2" + # Grove key from the qa-eval environment secret. Base URL is non-sensitive and + # stays a repo var. + GROVE_API_KEY: ${{ secrets.GROVE_API_KEY }} + GROVE_BASE_URL: ${{ vars.GROVE_BASE_URL }} + steps: + # Missing model credentials fail loud in harness.py's preflight (which names the + # missing vars) — not here. This job must not be enabled (uncomment the push/schedule + # triggers) until the GitHub 'qa-eval' environment exists with GROVE_API_KEY + # (secret) + GROVE_BASE_URL (repo var) (see mongodb/agent-skills#51 / ENTSEC-5465). + - name: Checkout agent-skills (this repo, public) + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 + with: + path: agent-skills + + - name: Generate a token for the agent-skills-evals GitHub App + id: app-token + uses: actions/create-github-app-token@fee1f7d63c2ff003460e3d139729b119787bc349 # v2.2.2 + with: + app-id: ${{ secrets.AGENT_SKILLS_EVALS_APP_ID }} + private-key: ${{ secrets.AGENT_SKILLS_EVALS_APP_PRIVATE_KEY }} + owner: 10gen + repositories: agent-skills-evals + + - name: Checkout agent-skills-evals (private, sibling checkout) + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 + with: + repository: 10gen/agent-skills-evals + token: ${{ steps.app-token.outputs.token }} + path: agent-skills-evals + + - uses: astral-sh/setup-uv@38f3f104447c67c051c4a08e39b64a148898af3a # v4 + with: + python-version: "3.12" + + - name: Run baseline + every mutant for ${{ matrix.skill }} + working-directory: agent-skills-evals/inspect + run: | + set -euo pipefail + uv sync + + echo "== BASELINE (unmutated skill, skill-on) at k=$EPOCHS ==" + uv run python harness.py \ + --skill "${{ matrix.skill }}" --epochs "$EPOCHS" --max-samples "$MAX_SAMPLES" + + echo "== MUTATION BATTERY ==" + MUT_DIR=$(mktemp -d) + uv run python mutate_skill.py --skill "${{ matrix.skill }}" --all --out "$MUT_DIR" + for d in "$MUT_DIR"/mut-*; do + [ -d "$d" ] || continue + echo "-- $(basename "$d") --" + SKILL_BASE_DIR="$d" uv run python harness.py \ + --skill "${{ matrix.skill }}" --epochs "$EPOCHS" --max-samples "$MAX_SAMPLES" + done + + echo "== REPORT ==" + uv run python mutation_report.py --skill "${{ matrix.skill }}" --json \ + > "$RUNNER_TEMP/${{ matrix.skill }}-mutation.json" + + # Reports are collected as artifacts and committed ONCE by `publish` below. Committing + # here would have every matrix job push to agent-skills-evals main concurrently: the + # first wins, the rest fail on non-fast-forward, and with fail-fast:false a census + # would quietly lose most of its results. + - name: Upload the sensitivity report + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: mutation-baseline-${{ matrix.skill }} + path: ${{ runner.temp }}/${{ matrix.skill }}-mutation.json + if-no-files-found: error + + publish: + needs: battery + if: always() && needs.battery.result != 'skipped' + runs-on: ubuntu-latest + # Separate environment from the spend jobs: `publish` never calls Grove, so it must + # NOT be gated by qa-eval's required-reviewers (else every push/schedule auto-run + # stalls on approval). The App PEM is a repo-level secret, so no OIDC is needed here. + environment: qa-eval-publish + permissions: + contents: read + steps: + - name: Generate a token for the agent-skills-evals GitHub App + id: app-token + uses: actions/create-github-app-token@fee1f7d63c2ff003460e3d139729b119787bc349 # v2.2.2 + with: + app-id: ${{ secrets.AGENT_SKILLS_EVALS_APP_ID }} + private-key: ${{ secrets.AGENT_SKILLS_EVALS_APP_PRIVATE_KEY }} + owner: 10gen + repositories: agent-skills-evals + + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 + with: + repository: 10gen/agent-skills-evals + token: ${{ steps.app-token.outputs.token }} + + # Whatever succeeded gets published: a skill whose battery failed simply has no + # artifact, so a partial census still lands its usable half. + - name: Download every skill's report + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 + with: + pattern: mutation-baseline-* + path: incoming + merge-multiple: true + + - name: Commit updated baselines in one commit + run: | + set -euo pipefail + shopt -s nullglob + FILES=(incoming/*-mutation.json) + if [ ${#FILES[@]} -eq 0 ]; then + echo "No reports to publish (every battery failed)." + exit 0 + fi + mkdir -p baselines + cp "${FILES[@]}" baselines/ + rm -rf incoming + git config user.name "agent-skills-evals-bot" + git config user.email "noreply@mongodb.com" + git add baselines + if git diff --cached --quiet; then + echo "No change to any baseline." + exit 0 + fi + git commit -m "chore: refresh mutation baselines (${#FILES[@]} skill(s))" + # The expected loser-case is a concurrent human commit to main, not a conflict on + # these files, so rebase and retry rather than failing a several-hour census. A + # CONFLICT is different: it means another run published the same baseline, git is + # left mid-rebase (where every further `git pull --rebase` fails on state, not + # content, so retrying can never recover), and a human should decide which + # measurement stands. Abort the rebase and fail loudly instead. + for attempt in 1 2 3; do + if git pull --rebase --autostash origin HEAD && git push; then + exit 0 + fi + if git rebase --abort 2>/dev/null; then + echo "::error::Rebase conflict publishing baselines — another run likely updated the same file. Re-run this battery to regenerate from current main." + exit 1 + fi + echo "push attempt $attempt failed; retrying" + sleep $((attempt * 5)) + done + exit 1 diff --git a/.github/workflows/qa-pr-screen.yml b/.github/workflows/qa-pr-screen.yml new file mode 100644 index 0000000..bf64750 --- /dev/null +++ b/.github/workflows/qa-pr-screen.yml @@ -0,0 +1,218 @@ +name: QA PR Screen Check + +# Advisory per-PR screen, run on demand — workflow_dispatch only. +# +# What this does: for one skill, runs the skill-off then skill-on arm ONCE (k=1 — this is +# a screen, not a measurement; more epochs would not help a per-PR decision), then posts a +# PR comment built by the QA harness's screen_report.py (10gen/agent-skills-evals). That +# script allowlists what may be posted — allowlisting, not scrubbing, is what makes +# commenting safe on a public PR — and suppresses the lift line when the two arms are not +# comparable or either is inconclusive, rather than publishing a number that is mostly an +# environment difference. +# +# Advisory only: it comments, never blocks. The blocking gate is qa-eval.yml. This +# workflow also does not duplicate the schema/lint checks in validate-eval-cases.yml. +# +# Why there is no `pull_request` trigger: +# - FORK PRs get no access to the `qa-eval` environment secrets, so every external +# contributor's PR would get a screen job that can't reach the model API — an +# intimidating red X on a first-time contribution, for a check that is explicitly +# advisory. +# - Same-repo `pull_request` would need the untrusted PR-head checkout and the Grove +# key in the same workflow. skill-gate (#51, ENTSEC-5465) handles that with a +# collect/run job split (unprivileged checkout job, privileged runner that receives +# the content as an artifact). That split previously lived in this file and was +# REMOVED with the trigger: it only means anything when a pull_request trigger +# exists, and until then it was untestable dead weight. If a pull_request trigger is +# ever enabled, restore the split from git history FIRST — and note the limit the +# old header documented: the split keeps the key off the runner that saw the +# untrusted ref, but the harness still executes untrusted eval-case YAML with the +# key in its env. Full isolation would need model-call-layer sandboxing. +# +# The dispatch ref is operator-chosen and treated as trusted, so this single job checks +# out and runs directly — no split. When pr_number is given the dispatch ref is IGNORED: +# the PR's current head SHA is resolved and checked out instead, the evaluated SHA is +# stamped into the comment, and the post step re-resolves the head and refuses to post +# if it moved. A comment on a PR must report on that PR's code, never on an +# operator-picked ref posted as though the PR had been screened. +# +# Prereqs (see qa-eval.yml's header, ENTSEC-5465): +# - GitHub environment `qa-eval` with secret GROVE_API_KEY +# - repo variable GROVE_BASE_URL (non-sensitive) +# - repo secrets AGENT_SKILLS_EVALS_APP_ID + AGENT_SKILLS_EVALS_APP_PRIVATE_KEY +# - 10gen/agent-skills-evals#18 merged (provides harness.py + screen_report.py). +# Missing model credentials fail loud there (harness.py preflight), not here. + +on: + workflow_dispatch: + inputs: + skill: + description: "Skill to screen" + required: true + type: string + pr_number: + description: "PR number to comment on (leave blank to just print the comment body)" + required: false + type: string + +# One screen per skill. Two dispatches in quick succession would otherwise race on the +# same marker comment and can double-post instead of updating. +concurrency: + group: qa-pr-screen-${{ github.event.inputs.skill }} + cancel-in-progress: true + +permissions: + contents: read + pull-requests: write + +env: + # Kept in lockstep with harness.py's and screen_report.py's default, and passed + # explicitly to both. screen_report reads the LATEST qa_runs doc for --skill/--model, so + # a drift between the two defaults doesn't produce a wrong number — it produces "no + # skill-on qa_runs doc found" and a failure nobody can read. + PANEL_MODEL: anthropic/claude-sonnet-4-6 + +jobs: + screen: + runs-on: ubuntu-latest + # Holds GROVE_API_KEY as an environment secret (gated behind required reviewers). + # Base URL is a non-sensitive repo var. + environment: qa-eval + timeout-minutes: 60 + services: + mongo: + image: mongo:7 + ports: + - 27017:27017 + options: >- + --health-cmd "mongosh --eval 'db.runCommand({ping:1})'" + --health-interval 5s --health-timeout 5s --health-retries 10 + env: + MONGODB_URI: mongodb://localhost:27017 + MONGODB_DB: qa_pr_screen + # A CONCURRENCY cap, not a case count — see qa-mutation-battery.yml's MAX_SAMPLES + # note on why 4 is too many agent+mongo container pairs for a 2-vCPU runner. + MAX_SAMPLES: "2" + GROVE_API_KEY: ${{ secrets.GROVE_API_KEY }} + GROVE_BASE_URL: ${{ vars.GROVE_BASE_URL }} + steps: + # When posting to a PR, the evaluated code must BE that PR's head, so resolve it + # rather than trusting the dispatch ref. Without pr_number the dispatch ref stands + # (a print-only run that attributes nothing to any PR). + - name: Resolve the PR head SHA + id: pr-head + if: github.event.inputs.pr_number != '' + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.inputs.pr_number }} + run: | + set -euo pipefail + SHA=$(gh api "repos/${{ github.repository }}/pulls/$PR_NUMBER" --jq '.head.sha') + echo "sha=$SHA" >> "$GITHUB_OUTPUT" + + - name: Checkout agent-skills (PR head when posting, else the dispatch ref's SHA) + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 + with: + path: agent-skills + ref: ${{ steps.pr-head.outputs.sha || github.sha }} + + # A typo'd skill name must fail loudly here, not deep inside the harness on a skill + # that does not exist. + - name: Validate the requested skill exists + env: + SKILL: ${{ github.event.inputs.skill }} + REF: ${{ steps.pr-head.outputs.sha || github.sha }} + run: | + set -euo pipefail + if [ ! -f "agent-skills/skills/$SKILL/SKILL.md" ]; then + echo "::error::No skill named '$SKILL' under skills/ (on $REF)." + ls agent-skills/skills/ + exit 1 + fi + + - name: Generate a token for the agent-skills-evals GitHub App + id: app-token + uses: actions/create-github-app-token@fee1f7d63c2ff003460e3d139729b119787bc349 # v2.2.2 + with: + app-id: ${{ secrets.AGENT_SKILLS_EVALS_APP_ID }} + private-key: ${{ secrets.AGENT_SKILLS_EVALS_APP_PRIVATE_KEY }} + owner: 10gen + repositories: agent-skills-evals + + - name: Checkout agent-skills-evals (private, sibling checkout) + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 + with: + repository: 10gen/agent-skills-evals + token: ${{ steps.app-token.outputs.token }} + path: agent-skills-evals + + - uses: astral-sh/setup-uv@38f3f104447c67c051c4a08e39b64a148898af3a # v4 + with: + python-version: "3.12" + + - name: Run skill-off then skill-on (k=1 — this is a screen, not a measurement) + working-directory: agent-skills-evals/inspect + env: + SKILL: ${{ github.event.inputs.skill }} + run: | + set -euo pipefail + uv sync + uv run python harness.py --skill "$SKILL" --no-skill \ + --model "$PANEL_MODEL" --epochs 1 --max-samples "$MAX_SAMPLES" + uv run python harness.py --skill "$SKILL" \ + --model "$PANEL_MODEL" --epochs 1 --max-samples "$MAX_SAMPLES" + + - name: Build the sanitized PR comment + working-directory: agent-skills-evals/inspect + env: + SKILL: ${{ github.event.inputs.skill }} + run: | + uv run python screen_report.py --skill "$SKILL" \ + --model "$PANEL_MODEL" > comment.md + + # The comment (and the job summary) must name the exact SHA that was screened, so a + # result is attributable to the code it measured. Appended at the END: the first line + # stays screen_report.py's marker, which the post step below matches on. SHA + skill + # name are self-generated fields, squarely inside the public-content rule. + - name: Stamp the evaluated SHA into the comment + working-directory: agent-skills-evals/inspect + env: + EVALUATED_SHA: ${{ steps.pr-head.outputs.sha || github.sha }} + SKILL: ${{ github.event.inputs.skill }} + run: | + printf '\n---\nEvaluated SHA: `%s` (skill `%s`)\n' "$EVALUATED_SHA" "$SKILL" >> comment.md + + - name: Show the comment in the job summary + run: cat "agent-skills-evals/inspect/comment.md" >> "$GITHUB_STEP_SUMMARY" + + - name: Post or update the PR comment + if: github.event.inputs.pr_number != '' + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.inputs.pr_number }} + SKILL: ${{ github.event.inputs.skill }} + EVALUATED_SHA: ${{ steps.pr-head.outputs.sha }} + run: | + set -euo pipefail + # Re-resolve the head before posting: if the PR moved since the eval started, + # this comment would attribute a result to code it did not measure. + CURRENT=$(gh api "repos/${{ github.repository }}/pulls/$PR_NUMBER" --jq '.head.sha') + if [ "$CURRENT" != "$EVALUATED_SHA" ]; then + echo "::error::PR head moved during the run ($EVALUATED_SHA -> $CURRENT). Not posting; re-run the screen." + exit 1 + fi + # screen_report.py emits this marker as the comment's first line + # (screen_report.marker(), asserted in test_screen_report.py), which is what lets + # successive runs update one comment instead of stacking new ones. + MARKER="" + EXISTING=$(gh api "repos/${{ github.repository }}/issues/$PR_NUMBER/comments" --paginate \ + --jq ".[] | select(.body | startswith(\"$MARKER\")) | .id" | head -1) + # -F body=@file, not -f body="$(cat ...)": the comment is multi-line markdown and + # passing it through a shell interpolation is a needless quoting hazard. + if [ -n "$EXISTING" ]; then + gh api "repos/${{ github.repository }}/issues/comments/$EXISTING" -X PATCH \ + -F body=@agent-skills-evals/inspect/comment.md + else + gh api "repos/${{ github.repository }}/issues/$PR_NUMBER/comments" -X POST \ + -F body=@agent-skills-evals/inspect/comment.md + fi diff --git a/.github/workflows/validate-eval-cases.yml b/.github/workflows/validate-eval-cases.yml new file mode 100644 index 0000000..9f7c080 --- /dev/null +++ b/.github/workflows/validate-eval-cases.yml @@ -0,0 +1,59 @@ +name: Validate Eval Cases + +# Schema-validate testing//evals/evals.json against testing/evals.schema.json (also +# checks referenced `files`/`seed` assets actually exist) so a malformed or broken-pathed eval +# case fails where it's authored, not later in the QA harness. Also lints for items that echo +# their own skill's guidance text (advisory: reports, does not block — see the --strict TODO +# in testing/lint-item-echo.mjs). Scoped to eval cases, skills, and the schema so it only +# runs when one of those changes. +on: + pull_request: + paths: + - "testing/**/evals/evals.json" + - "testing/evals.schema.json" + - "testing/validate-evals.mjs" + - "testing/validate-evals.test.mjs" + - "testing/lint-item-echo.mjs" + - "testing/lint-item-echo.test.mjs" + - "testing/echo-thresholds.json" + - "testing/package.json" + # The lockfile too: `npm ci` installs from it, so a lockfile-only change (a dependency + # bump, a regen) can change what this workflow actually runs. Without it here, an ajv + # upgrade that breaks validation would sail past the very check meant to catch it. + - "testing/package-lock.json" + - "skills/**/SKILL.md" + - "skills/**/references/**" + +permissions: + contents: read + +jobs: + validate-eval-cases: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 + with: + # Co-movement needs the base commit's SKILL.md to compare against. + fetch-depth: 0 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 + with: + # Match the repo's other Node workflows (24): a validator that runs a different + # major than everything else can disagree with local runs for version reasons. + node-version: "24" + - name: Install validator deps + run: npm ci --prefix testing --no-audit --no-fund + - name: Validate eval cases against schema, check referenced assets exist + run: node testing/validate-evals.mjs + - name: Pin the files-containment rule and the echo-lint's Python parity + # validate-evals.test.mjs pins the cross-skill answer-key handover guard; + # lint-item-echo.test.mjs pins the tokenizer/span behaviour that keeps this lint a + # true port of agent-skills-evals's analysis/echo.py (same fixtures and expected + # values as test_echo.py — the two repos' CIs can't import each other, so both pin + # the same numbers). + run: node --test testing/validate-evals.test.mjs testing/lint-item-echo.test.mjs + - name: Lint for items that echo their own skill's guidance text (advisory) + # --base enables the co-movement check: an edit to SKILL.md that RAISES an eval + # item's overlap with SKILL.md is teaching to the test, and it is detectable from + # the diff alone -- no held-out corpus and no curator needed. Without --base only + # the weaker static checks run. + run: node testing/lint-item-echo.mjs --base "${{ github.event.pull_request.base.sha }}" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 31be944..2220b9c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -73,6 +73,20 @@ directly avoids all of that. GitHub's pull-request files API, which caps out there; beyond it, skill changes could be missed. A PR anywhere near that size should be split for reviewability anyway. +### Manually approving a QA eval + +Deleted skills and skills without a GitHub runner profile leave the required `qa-eval` +check in `action_required`. A maintainer may clear it only through the **QA Eval Manual +Approval** workflow. + +Provide the PR number, its exact current head SHA, a review reason, and an HTTPS evidence +link. The workflow rejects stale SHAs, unauthorized callers, and checks that are not +currently `action_required`. Its successful check summary records the reviewer and evidence. +Do not use an admin merge or an empty commit to bypass this review. + +Repository administrators must protect the `qa-eval-manual-review` environment with required +reviewers before enabling this workflow. + ## Releasing Releases are driven by GitHub Actions. Release operators should use the diff --git a/testing/echo-thresholds.json b/testing/echo-thresholds.json new file mode 100644 index 0000000..cc1a601 --- /dev/null +++ b/testing/echo-thresholds.json @@ -0,0 +1,131 @@ +{ + "_comment": "Answer-echo detection constants AND tokenizer definition, declared once here because two implementations consume them: testing/lint-item-echo.mjs (this repo, runs on every PR, no model calls, no private checkout) and agent-skills-evals/inspect/analysis/echo.py (the offline analysis layer). The ALGORITHM is implemented twice because the lint has to run on a public PR without cloning a private repo -- but it is a TRUE PORT: both sides tokenize identically (same regex, same fenced-code stripping, same stopword list from this file) and compute the same verbatim span, so the constants below mean the same quantity on both sides. The only deliberate difference is reporting granularity: the lint scores prompt and answer fields separately (leakage vs legitimate restatement read differently in review); the analysis layer pools them.", + "domainStopwords": [ + "a", + "an", + "and", + "are", + "as", + "at", + "be", + "by", + "for", + "from", + "has", + "have", + "in", + "into", + "is", + "it", + "its", + "of", + "on", + "or", + "that", + "the", + "to", + "with", + "you", + "your", + "this", + "these", + "those", + "if", + "then", + "when", + "use", + "using", + "used", + "should", + "must", + "can", + "will", + "would", + "may", + "mongodb", + "mongo", + "db", + "database", + "databases", + "collection", + "collections", + "document", + "documents", + "field", + "fields", + "index", + "indexes", + "indices", + "query", + "queries", + "aggregate", + "aggregation", + "pipeline", + "stage", + "stages", + "find", + "filter", + "sort", + "limit", + "skip", + "project", + "group", + "match", + "lookup", + "unwind", + "count", + "explain", + "executionstats", + "keypattern", + "ixscan", + "collscan", + "compound", + "single", + "equality", + "range", + "connection", + "connect", + "client", + "pool", + "poolsize", + "timeout", + "uri", + "driver", + "search", + "vector", + "embedding", + "atlas", + "cluster", + "mcp", + "server", + "tool", + "tools", + "skill", + "agent", + "schema", + "design", + "pattern", + "embed", + "reference", + "bucket", + "create", + "created", + "creates", + "make", + "made", + "get", + "set", + "add", + "added", + "remove" + ], + "n": 8, + "minAbsoluteContainment": 0.1, + "maxVerbatimSpan": 20, + "coMovementMinDelta": 0.05, + "_thresholdNotes": { + "minAbsoluteContainment": "The containment floor at which an item is flagged as echoing its own skill. This constant IS the threshold: an earlier version also required exceeding a percentile of a cross-skill null, but measured on the real corpus the null collapses to 0.000 (two skills essentially never share an 8-gram), so the percentile leg could never change an outcome and was removed from both implementations.", + "maxVerbatimSpan": "A shared run this long is a quotation regardless of the containment score.", + "coMovementMinDelta": "Containment rise, within one PR, that counts as teaching to the test." + } +} diff --git a/testing/evals.schema.json b/testing/evals.schema.json new file mode 100644 index 0000000..225209e --- /dev/null +++ b/testing/evals.schema.json @@ -0,0 +1,417 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://github.com/mongodb/agent-skills/testing/evals.schema.json", + "title": "Agent-skills QA eval cases (extended)", + "description": "Schema for agent-skills/testing//evals/evals.json. Extends the current judge-only shape (prompt/expected_output/expectations/files) with OPTIONAL functional-grading fields (seed, functional_check, trajectory_checks) so the QA harness can grade against a real MongoDB. All functional fields are optional and additive: existing knowledge-only cases validate unchanged. Fixtures referenced by `seed` live in the sibling `fixtures/` directory as deterministic .js scripts.", + "x-fixtureContract": { + "description": "The seed/fixture naming contract, declared here because it is shared across repos: agent-skills's validate-evals.mjs enforces it at authoring time and agent-skills-evals's solvers/seed_db.py resolves against it at run time. Both read these values rather than keeping their own copy, so the two cannot drift into a state where a case passes CI here and then fails to seed inside the harness.", + "fixtureDir": "fixtures", + "fixtureExtension": ".js", + "reservedSeeds": [ + "clean_slate" + ] + }, + "type": "object", + "required": [ + "skill_name", + "evals" + ], + "additionalProperties": false, + "properties": { + "skill_name": { + "type": "string", + "description": "Skill directory name, e.g. mongodb-query-optimizer.", + "pattern": "^[a-z0-9-]+$" + }, + "evals": { + "type": "array", + "minItems": 1, + "items": { + "$ref": "#/$defs/case" + } + } + }, + "$defs": { + "case": { + "type": "object", + "required": [ + "id" + ], + "additionalProperties": false, + "patternProperties": { + "^_": { + "description": "Author comment key (e.g. _note); ignored by the harness." + } + }, + "description": "One test case. Single-turn cases use `prompt`; multi-turn cases use `workflow`.", + "properties": { + "id": { + "type": "integer", + "description": "Stable case id (unique within the skill; used to correlate results across runs)." + }, + "name": { + "type": "string", + "description": "Optional human-readable case label." + }, + "prompt": { + "type": "string", + "description": "The task given to the agent (single-turn). Required unless `workflow` is present." + }, + "workflow": { + "type": "array", + "description": "Multi-turn task: ordered turns. Alternative to `prompt`.", + "minItems": 1, + "items": { + "type": "object", + "required": [ + "turn", + "prompt" + ], + "additionalProperties": false, + "properties": { + "turn": { + "type": "integer", + "minimum": 1 + }, + "prompt": { + "type": "string" + } + } + } + }, + "expected_output": { + "type": "string", + "description": "Narrative description of a correct answer. Lowest-priority judge target (used if no metadata assertions and no expectations)." + }, + "expectations": { + "type": "array", + "description": "Skill-level checkable criteria. Judge target when present and no per-case metadata assertions exist.", + "items": { + "type": "string" + } + }, + "files": { + "type": "array", + "description": "Relative paths (under the evals dir) of asset files inlined into the prompt. Must stay INSIDE the evals dir: no leading '/' and no '..' segment. This is an eval-integrity rule, not a filesystem one -- the harness inlines these into the prompt (case_source.build_prompt), and the answer-echo lint deliberately excludes `files` from its overlap analysis because they are provided input data. A case that escaped upward to its own skills//SKILL.md would hand the agent the answer key with nothing flagging it.", + "items": { + "type": "string", + "pattern": "^(?!/)(?!.*(^|/)\\.\\.(/|$)).+$" + } + }, + "seed": { + "type": "string", + "description": "Names a DB fixture in the sibling fixtures/ dir (fixtures/.js), applied to the sandbox MongoDB before the agent runs. 'clean_slate' = empty DB (no fixture file). Required for any functional_check that reads real data.", + "pattern": "^[a-z0-9_]+$" + }, + "functional_check": { + "$ref": "#/$defs/functional_check" + }, + "trajectory_checks": { + "type": "array", + "description": "Deterministic tool-use assertions on the agent's trajectory.", + "items": { + "$ref": "#/$defs/trajectory_check" + } + } + }, + "oneOf": [ + { + "required": [ + "prompt" + ], + "not": { + "required": [ + "workflow" + ] + } + }, + { + "required": [ + "workflow" + ], + "not": { + "required": [ + "prompt" + ] + } + } + ] + }, + "functional_check": { + "type": "object", + "description": "Grades the agent's produced query/index against the seeded DB by RE-EXECUTING it (not trusting the agent's text). Discriminated by `type`.", + "required": [ + "type", + "database", + "collection" + ], + "properties": { + "type": { + "enum": [ + "aggregation", + "index", + "explain" + ] + }, + "database": { + "type": "string" + }, + "collection": { + "type": "string" + } + }, + "oneOf": [ + { + "title": "aggregation \u2014 re-run the agent's pipeline, compare returned docs", + "required": [ + "type", + "artifact", + "expected" + ], + "additionalProperties": false, + "properties": { + "type": { + "const": "aggregation" + }, + "database": { + "type": "string" + }, + "collection": { + "type": "string" + }, + "artifact": { + "type": "string", + "description": "Path the agent writes its JSON pipeline array to, e.g. /workspace/pipeline.json." + }, + "expected": { + "type": "array", + "description": "Expected result documents (subset of fields compared).", + "items": { + "type": "object" + } + }, + "compare_fields": { + "type": "array", + "description": "If set, compare only these keys of each doc.", + "items": { + "type": "string" + } + }, + "ordered": { + "type": "boolean", + "default": true, + "description": "Whether result order must match." + } + } + }, + { + "title": "index \u2014 assert an index with the expected key exists", + "required": [ + "type", + "expected_key" + ], + "additionalProperties": false, + "properties": { + "type": { + "const": "index" + }, + "database": { + "type": "string" + }, + "collection": { + "type": "string" + }, + "expected_key": { + "description": "An index key spec, OR a list of acceptable specs (any match passes \u2014 e.g. an ESR index and its reverse-scan inverse).", + "oneOf": [ + { + "type": "object" + }, + { + "type": "array", + "items": { + "type": "object" + }, + "minItems": 1 + } + ] + } + } + }, + { + "title": "explain \u2014 grade the query PLAN (robust to which optimal index was built)", + "required": [ + "type", + "filter" + ], + "additionalProperties": false, + "properties": { + "type": { + "const": "explain" + }, + "database": { + "type": "string" + }, + "collection": { + "type": "string" + }, + "filter": { + "type": "object", + "description": "Query predicate for explain()." + }, + "sort": { + "type": "object", + "description": "Optional sort spec." + }, + "require_ixscan": { + "type": "boolean", + "default": true, + "description": "Winning plan must use an index (no COLLSCAN)." + }, + "forbid_in_memory_sort": { + "type": "boolean", + "default": true, + "description": "No blocking SORT stage." + }, + "max_docs_examined_ratio": { + "type": "number", + "exclusiveMinimum": 0, + "description": "Fail if totalDocsExamined > nReturned * ratio (catches over-scan)." + } + } + } + ] + }, + "matcher": { + "type": "object", + "description": "Selects tool calls in the trajectory. Combine fields (all must hold).", + "minProperties": 1, + "additionalProperties": false, + "properties": { + "any_mcp": { + "type": "boolean", + "description": "Match any MCP tool call (mcp____)." + }, + "tool": { + "type": "string", + "description": "Bare tool name (namespace-agnostic; e.g. 'aggregate', not 'mcp__mongodb__aggregate')." + }, + "where": { + "type": "object", + "description": "Param equalities the call's arguments must satisfy (path -> value)." + }, + "success": { + "type": "boolean", + "description": "Require the matched call's joined result to have succeeded (true) or failed (false). Implied by type=called_successfully." + } + } + }, + "trajectory_check": { + "type": "object", + "required": [ + "type" + ], + "description": "A mechanical tool-use assertion. `critical: true` gates the score; non-critical are recorded only.", + "properties": { + "id": { + "type": "string" + }, + "critical": { + "type": "boolean", + "default": false + }, + "type": { + "enum": [ + "called", + "absent", + "order", + "param", + "called_successfully" + ], + "description": "called = the call was ATTEMPTED (passes even if it errored); called_successfully = the call was attempted AND returned without error. Prefer called_successfully for any gating check: an attempted call can still have failed (e.g. the MCP server never registered and every call returned an error), and counting attempts alone reads that as healthy." + } + }, + "oneOf": [ + { + "title": "called / called_successfully / absent \u2014 a matching call must be present (optionally succeeded) / absent", + "required": [ + "type", + "match" + ], + "additionalProperties": false, + "properties": { + "id": { + "type": "string" + }, + "critical": { + "type": "boolean" + }, + "type": { + "enum": [ + "called", + "called_successfully", + "absent" + ] + }, + "match": { + "$ref": "#/$defs/matcher" + } + } + }, + { + "title": "order \u2014 every `first` match precedes every `then` match", + "required": [ + "type", + "first", + "then" + ], + "additionalProperties": false, + "properties": { + "id": { + "type": "string" + }, + "critical": { + "type": "boolean" + }, + "type": { + "const": "order" + }, + "first": { + "$ref": "#/$defs/matcher" + }, + "then": { + "$ref": "#/$defs/matcher" + } + } + }, + { + "title": "param \u2014 the matched call's arguments satisfy every `expect` equality", + "required": [ + "type", + "match", + "expect" + ], + "additionalProperties": false, + "properties": { + "id": { + "type": "string" + }, + "critical": { + "type": "boolean" + }, + "type": { + "const": "param" + }, + "match": { + "$ref": "#/$defs/matcher" + }, + "expect": { + "type": "object", + "description": "Dotted-path -> value equalities the matched call's arguments must satisfy. Separate from `match` so the call SELECTION (`match`) stays independent of the VALUES asserted (`expect`). Required: a param check that asserts nothing is a check that cannot fail." + } + } + } + ] + } + } +} diff --git a/testing/lint-item-echo.mjs b/testing/lint-item-echo.mjs new file mode 100644 index 0000000..8e6b04b --- /dev/null +++ b/testing/lint-item-echo.mjs @@ -0,0 +1,372 @@ +#!/usr/bin/env node +/** + * Flag eval items that echo their own skill's guidance text rather than testing whether + * an agent can produce it unaided. An item authored by copy-pasting from SKILL.md or a + * references/*.md file is grading "can the agent quote the instructions back," not + * "did the agent apply them" — an easy mistake when the skill's author also writes its + * tests, with no held-out curator to catch it. + * + * Three checks, in increasing order of how much they can actually prove: + * + * 1. **Static containment** (all items). 8-word-gram containment of the item's authored + * text against its own skill's body, flagged at or above an absolute floor + * (minAbsoluteContainment). Containment, not Jaccard, because the item is always much + * shorter than the skill body, so Jaccard would be swamped by that length asymmetry + * regardless of overlap. An earlier version also gated on a cross-skill null + * percentile; measured on this corpus the null collapses to 0.000 (two skills + * essentially never share an 8-gram), so that leg could never change an outcome and + * was removed — the floor is the threshold. + * + * Reported per field, not pooled, because the two fields mean different things. High + * overlap in `prompt` is leakage: the question is carrying its own answer. High overlap + * in `expected_output`/`expectations` is often unavoidable and fine — if the correct + * answer IS the rule, an item that states the rule is correctly authored. Pooling them + * would mostly measure the second and call it the first. + * + * 2. **Verbatim span.** A shared run of ≥ maxVerbatimSpan tokens is a quotation whatever + * the containment score says. Computed as the true longest common token run (the rule + * that must never false-positive), over RAW tokens — a quotation includes the ordinary + * words, and stripping them would fragment the run and understate the copy. + * + * 3. **Co-movement** (`--base `, the check that needs no held-out set). If a PR edits + * a skill's guidance AND that edit raises an item's containment, that is teaching to the + * test — detectable from the diff alone, no curator and no golden corpus required. This + * is the highest-value check here and it is pure string processing. + * + * echo-thresholds.json is the complete shared contract with the Python analysis layer + * (agent-skills-evals/inspect/analysis/echo.py): the constants AND the tokenizer + * definition (regex, fenced-code stripping, stopword list). This file is a TRUE PORT — + * both sides compute the same quantities, so the shared constants mean the same thing. + * The one deliberate difference is reporting granularity: the lint scores `prompt` and + * answer fields separately (leakage vs legitimate restatement read differently in + * review); the Python module pools them. + * + * Advisory today (prints, exits 0) unless --strict is passed. + * TODO: flip validate-eval-cases.yml to --strict once the thresholds have been checked + * against a few real PRs of item additions, or record why advisory is the permanent + * answer. + */ +import { execFileSync } from "node:child_process"; +import { readFileSync } from "node:fs"; +import { dirname, join, relative } from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { globSync } from "glob"; + +const here = dirname(fileURLToPath(import.meta.url)); +const repoRoot = join(here, ".."); + +function readJson(file) { + const text = readFileSync(file, "utf8"); + try { + return JSON.parse(text); + } catch (err) { + console.error(`✗ ${file}\n not valid JSON: ${err.message}`); + process.exit(1); + } +} + +const T = readJson(join(here, "echo-thresholds.json")); +const N = T.n; +const MIN_CONTAINMENT = T.minAbsoluteContainment; +const MAX_SPAN = T.maxVerbatimSpan; +const CO_MOVEMENT_MIN_DELTA = T.coMovementMinDelta; + +// In GitHub Actions, surface findings as PR annotations (file-attached ::warning) so they +// appear in the Files Changed review view even while the lint is advisory (exit 0). A stdout +// line in a green workflow log is invisible to a reviewer; an annotation is not. The lint +// stays non-blocking — these are ::warning, not ::error — matching the advisory intent (see +// the --strict TODO above). Locally (no GITHUB_ACTIONS), keep the human-readable line so +// `node lint-item-echo.mjs` output is unchanged. +const IN_CI = process.env.GITHUB_ACTIONS === "true"; + +/** + * Report one finding. `file` is the evals.json path; `message` is a one-line human message. + * Emits a GitHub `::warning file=…::` workflow command in CI (attaching to the file in PR + * review) and the `⚠ …` line in both environments. + */ +function reportFinding(file, message) { + const rel = relative(repoRoot, file); + if (IN_CI) { + // `file` is required for the annotation to attach; line/column are omitted (the lint + // works on whole-file JSON, not source positions). Percent/newline escaping per the + // workflow-command spec; `message` is single-line by construction here. + const escaped = message.replace(/%/g, "%25").replace(/\n/g, "%0A").replace(/\r/g, "%0D"); + process.stdout.write(`::warning file=${rel}::${escaped}\n`); + } + console.log(`⚠ ${rel}: ${message}`); +} + +// The stopword list is part of the shared contract (echo-thresholds.json), not a local +// choice: two implementations with different tokenizers compute different containment +// values for the SAME shared floor, which is precisely the disagreement the file exists +// to prevent. Domain vocabulary (index, collection, schema, …) is stripped — those are +// the words an item MUST use to describe its task, so leaving them in makes legitimate +// items look like copies. +const STOPWORDS = new Set(T.domainStopwords ?? []); + +/** + * True port of agent-skills-evals/inspect/analysis/echo.py::tokenize — same fenced-code + * stripping, same regex, same sigil/hyphen normalisation, same stopword list. Fenced code + * blocks are stripped first: code examples in a skill are not prose to quote, and an item + * restating a code block is legitimate reuse, not echo. Operator-ish tokens keep their + * sigil through the split so they can be matched against the stopword list without it, + * which is why the normalisation is a second pass. + */ +export function tokenize(text, { stripDomain = true } = {}) { + const noFences = (text ?? "").toLowerCase().replace(/```[\s\S]*?```/g, " "); + const raw = noFences.match(/[a-z_$][a-z0-9_$-]*/g) ?? []; + const tokens = raw + .map((t) => t.replace(/^\$+/, "").replace(/^[-_]+|[-_]+$/g, "")) + .filter(Boolean); + return stripDomain ? tokens.filter((t) => !STOPWORDS.has(t)) : tokens; +} + +export function ngrams(tokens, n) { + const grams = new Set(); + for (let i = 0; i + n <= tokens.length; i++) { + grams.add(tokens.slice(i, i + n).join(" ")); + } + return grams; +} + +// containment = fraction of the ITEM's n-grams that also appear in the corpus +export function containment(itemGrams, corpusGrams) { + if (itemGrams.size === 0) return 0; + let hits = 0; + for (const g of itemGrams) if (corpusGrams.has(g)) hits++; + return hits / itemGrams.size; +} + +/** + * True longest run of consecutive tokens present in both texts — the verbatim-span rule + * is the one that must never false-positive, so this is the exact algorithm echo.py's + * longest_verbatim_span uses (binary search over run length over rolling k-gram sets), + * NOT a consecutive-n-gram-run approximation, which can stitch a "span" from matches at + * different corpus locations. + */ +export function longestVerbatimSpan(a, b) { + if (a.length === 0 || b.length === 0) return 0; + const sharesRun = (k) => { + if (k <= 0 || a.length < k || b.length < k) return false; + const bRuns = new Set(); + for (let i = 0; i + k <= b.length; i++) bRuns.add(b.slice(i, i + k).join(" ")); + for (let i = 0; i + k <= a.length; i++) { + if (bRuns.has(a.slice(i, i + k).join(" "))) return true; + } + return false; + }; + if (!sharesRun(1)) return 0; + let lo = 1; + let hi = Math.min(a.length, b.length); + while (lo < hi) { + const mid = Math.floor((lo + hi + 1) / 2); + if (sharesRun(mid)) lo = mid; + else hi = mid - 1; + } + return lo; +} + +/** Every markdown file that makes up a skill's guidance surface, as repo-relative paths. */ +function skillCorpusFiles(skillName) { + const dir = join(repoRoot, "skills", skillName); + // Sorted recursive glob, matching the Python side's sorted(rglob("*.md")) — file order + // shifts boundary n-grams, so it is part of the port, not a nicety. + return [join(dir, "SKILL.md"), ...globSync(join(dir, "references", "**", "*.md")).sort()]; +} + +function readCorpus(files) { + const parts = []; + for (const f of files) { + try { + parts.push(readFileSync(f, "utf8")); + } catch { + /* a missing SKILL.md is validate-skills.yml's failure to report, not ours */ + } + } + return parts.join("\n\n"); +} + +/** + * The same files as they were at `ref`. + * + * Returns `anyPresent` alongside the text because "" is ambiguous and the two readings call + * for opposite behaviour: a file that existed and was empty really is a before-state to + * compare against, whereas a skill that did not exist at base has no before-state at all. + * Co-movement asks "did this PR's edit move an item's overlap", and for a brand-new skill + * nothing moved -- every item would trivially measure 0 → n and get reported as a rise that + * never happened. An item that copies a NEW skill's text is still echoing, but that is the + * static check's finding to report, in its own words. + */ +function readCorpusAtRef(files, ref) { + const parts = []; + let anyPresent = false; + for (const f of files) { + try { + parts.push( + execFileSync("git", ["show", `${ref}:${relative(repoRoot, f)}`], { + cwd: repoRoot, + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + }), + ); + anyPresent = true; + } catch { + /* not present at base -- a newly added reference file contributes nothing "before" */ + } + } + return { text: parts.join("\n\n"), anyPresent }; +} + +/** + * The item's authored text, split by role. `files` is excluded from both: it is provided + * input data, not authored description, and an asset that legitimately quotes the docs is + * not an item echoing its skill. + */ +function itemFields(ev) { + const prompt = [ev.prompt ?? "", ...(ev.workflow ?? []).map((t) => t.prompt ?? "")].join("\n\n"); + const answer = [ev.expected_output ?? "", ...(ev.expectations ?? [])].join("\n\n"); + return { prompt, answer }; +} + +function main() { + const args = process.argv.slice(2); + const STRICT = args.includes("--strict"); + const baseIdx = args.indexOf("--base"); + const BASE_REF = baseIdx !== -1 ? args[baseIdx + 1] : null; + + const evalFiles = globSync(join(here, "*/evals/evals.json")).sort(); + const bySkill = evalFiles.map((file) => { + const doc = readJson(file); + const skillName = doc.skill_name; + const corpusFiles = skillCorpusFiles(skillName); + const corpusText = readCorpus(corpusFiles); + // An empty corpus silently makes every containment 0 and every item "clean" -- the lint + // would report a green result precisely when it is measuring nothing. The usual cause is + // `skill_name` not matching a directory under skills/, which is a config error worth + // failing on rather than passing quietly. + if (corpusText.trim() === "") { + console.error( + `✗ ${file}: skill_name '${skillName}' has no readable guidance text under ` + + `skills/${skillName}/ (SKILL.md + references/*.md). Nothing could be compared, so ` + + `a "no echoing items" result here would be meaningless.`, + ); + process.exit(1); + } + return { + file, + skillName, + doc, + corpusFiles, + corpusGrams: ngrams(tokenize(corpusText), N), + corpusTokensRaw: tokenize(corpusText, { stripDomain: false }), + }; + }); + + const perItem = []; + + for (const { file, skillName, doc, corpusGrams: ownCorpus } of bySkill) { + for (const ev of doc.evals ?? []) { + const fields = itemFields(ev); + const entry = { skillName, file, id: ev.id, fields: {} }; + for (const [role, text] of Object.entries(fields)) { + // No minimum-length skip: containment of an item too short to form an n-gram is 0 + // (not a fabricated score), and the verbatim-span rule must still see short items — + // a copied run long enough to trip maxVerbatimSpan on raw tokens can strip below N. + const tokens = tokenize(text); + entry.fields[role] = { + rawTokens: tokenize(text, { stripDomain: false }), + own: containment(ngrams(tokens, N), ownCorpus), + }; + } + perItem.push(entry); + } + } + + console.log( + `Flagging needs containment >= ${MIN_CONTAINMENT} against the item's own skill, ` + + `or a verbatim span >= ${MAX_SPAN} words.`, + ); + + // A prompt that quotes the guidance is leakage; an expected_output that does is often just + // a correctly-stated answer. Same numbers, different verdict, so they are reported apart. + const ROLE_NOTE = { + prompt: "LEAKAGE: the question carries its own answer", + answer: "expected answer restates the guidance (often legitimate — judge in review)", + }; + + let flagged = 0; + for (const item of perItem) { + const own = bySkill.find((s) => s.skillName === item.skillName); + for (const [role, m] of Object.entries(item.fields)) { + const span = longestVerbatimSpan(m.rawTokens, own.corpusTokensRaw); + const bySpan = span >= MAX_SPAN; + const byContainment = m.own >= MIN_CONTAINMENT; + if (!bySpan && !byContainment) continue; + flagged++; + const why = bySpan + ? `verbatim span of ${span} words shared with its own skill` + : `${(m.own * 100).toFixed(0)}% containment against ${item.skillName}`; + reportFinding(item.file, `case ${item.id} [${role}]: ${why} — ${ROLE_NOTE[role]}`); + } + } + + // Co-movement: did THIS PR's guidance edit raise an item's overlap with the guidance? + let coMoved = 0; + if (BASE_REF) { + for (const { file, skillName, doc, corpusFiles } of bySkill) { + const base = readCorpusAtRef(corpusFiles, BASE_REF); + const afterText = readCorpus(corpusFiles); + if (!base.anyPresent) { + console.log( + `Co-movement: skipping ${skillName} — no guidance at base, so this PR adds the ` + + `skill. Nothing moved; the static check above covers its items.`, + ); + continue; + } + const beforeText = base.text; + if (beforeText === afterText) continue; // guidance untouched in this PR + const beforeGrams = ngrams(tokenize(beforeText), N); + const afterGrams = ngrams(tokenize(afterText), N); + for (const ev of doc.evals ?? []) { + for (const [role, text] of Object.entries(itemFields(ev))) { + // No minimum-length skip: containment of an item too short to form an n-gram is + // 0, so its delta is 0 — same outcome, one less special case. + const grams = ngrams(tokenize(text), N); + const before = containment(grams, beforeGrams); + const after = containment(grams, afterGrams); + const delta = after - before; + if (delta < CO_MOVEMENT_MIN_DELTA) continue; + coMoved++; + reportFinding( + file, + `CO-MOVEMENT case ${ev.id} [${role}]: this PR's edit to ${skillName}'s ` + + `guidance raised containment ${before.toFixed(2)} → ${after.toFixed(2)} ` + + `(+${delta.toFixed(2)}). Added guidance text that overlaps an eval item is ` + + `teaching to the test, whatever the absolute number is.`, + ); + } + } + } + } else { + console.log( + "Co-movement check skipped (no --base ). It is the check that needs no held-out " + + "set, so pass the PR base SHA in CI.", + ); + } + + if (flagged === 0 && coMoved === 0) { + console.log("No echoing items found."); + } else { + console.log( + `\n${flagged} static finding(s), ${coMoved} co-movement finding(s). Not automatically ` + + "wrong — some overlap is expected for MongoDB vocabulary — but worth a second look: " + + "does the item test whether the agent APPLIES the guidance, or just whether it can " + + "quote it?", + ); + } + + process.exit(STRICT && flagged + coMoved > 0 ? 1 : 0); +} + +// Run as a CLI only when invoked directly, not when imported by the test. +if (import.meta.url === pathToFileURL(process.argv[1] ?? "").href) main(); diff --git a/testing/lint-item-echo.test.mjs b/testing/lint-item-echo.test.mjs new file mode 100644 index 0000000..2a6471f --- /dev/null +++ b/testing/lint-item-echo.test.mjs @@ -0,0 +1,108 @@ +// Pins the tokenizer/span behaviour that makes lint-item-echo.mjs a true port of +// agent-skills-evals/inspect/analysis/echo.py. The fixtures and expected values here are +// IDENTICAL to test_echo.py's — that is the cross-repo parity mechanism: neither repo's +// CI can import the other, so both pin the same numbers. +// +// Uses node's built-in test runner — no new dependency on the testing/ toolchain. +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { containment, longestVerbatimSpan, ngrams, tokenize } from "./lint-item-echo.mjs"; + +// Same fixtures as test_echo.py. +const SKILL = ` + Follow the ESR rule when building a compound index: equality fields first, then sort + fields, then range fields. A filter on an exact value with a descending sort should use + a compound index whose leading key is the equality field. +`; +const HONEST_ITEM = ` + Queries against the events collection that filter by an exact type and sort by + timestamp descending are slow. Diagnose the cause and create an appropriate index. +`; +const COPIED_ITEM = ` + Follow the ESR rule when building a compound index: equality fields first, then sort + fields, then range fields. A filter on an exact value with a descending sort should use + a compound index whose leading key is the equality field. +`; +const OTHER_SKILL_A = ` + Configure maxPoolSize to match expected concurrency. A serverless function should keep + the pool small and reuse the client across invocations rather than reconnecting. +`; + +const N = 8; // echo-thresholds.json's n + +function containmentOf(itemText, skillText) { + return containment(ngrams(tokenize(itemText), N), ngrams(tokenize(skillText), N)); +} + +test("tokenize: lowercases and splits", () => { + assert.ok(tokenize("ESR rule", { stripDomain: false }).includes("esr")); +}); + +test("tokenize: strips domain vocabulary by default", () => { + const toks = tokenize("create a compound index on the collection"); + assert.ok(!toks.includes("index") && !toks.includes("collection")); +}); + +test("tokenize: retains domain words when asked", () => { + assert.ok(tokenize("create a compound index", { stripDomain: false }).includes("index")); +}); + +test("tokenize: operator sigils normalised", () => { + assert.ok(tokenize("$match stage", { stripDomain: false }).includes("match")); +}); + +test("tokenize: hyphenated words stay one token", () => { + assert.deepEqual(tokenize("outer-join", { stripDomain: false }), ["outer-join"]); +}); + +test("tokenize: fenced code blocks are stripped", () => { + // code examples are not prose to quote; an item restating one is legitimate reuse + assert.deepEqual(tokenize("intro text\n```python\ndb.things.find({})\n```\nafter"), [ + "intro", + "text", + "after", + ]); +}); + +test("tokenize: empty text", () => { + assert.deepEqual(tokenize(""), []); +}); + +test("containment: identical text is fully contained", () => { + assert.equal(containmentOf(COPIED_ITEM, SKILL), 1.0); +}); + +test("containment: unrelated text is zero", () => { + assert.equal(containmentOf(HONEST_ITEM, OTHER_SKILL_A), 0.0); +}); + +test("containment: honest item scores below a copy", () => { + assert.ok(containmentOf(HONEST_ITEM, SKILL) < containmentOf(COPIED_ITEM, SKILL)); +}); + +test("containment: short item cannot be judged", () => { + assert.equal(containmentOf("create an index", SKILL), 0.0); +}); + +test("span: finds a long quotation", () => { + assert.ok( + longestVerbatimSpan( + tokenize(COPIED_ITEM, { stripDomain: false }), + tokenize(SKILL, { stripDomain: false }), + ) > 20, + ); +}); + +test("span: short for unrelated text", () => { + assert.ok( + longestVerbatimSpan( + tokenize(HONEST_ITEM, { stripDomain: false }), + tokenize(OTHER_SKILL_A, { stripDomain: false }), + ) < 5, + ); +}); + +test("span: zero on empty", () => { + assert.equal(longestVerbatimSpan([], tokenize(SKILL, { stripDomain: false })), 0); +}); diff --git a/testing/mongodb-natural-language-querying/evals/evals.json b/testing/mongodb-natural-language-querying/evals/evals.json index c028b48..2c87d91 100644 --- a/testing/mongodb-natural-language-querying/evals/evals.json +++ b/testing/mongodb-natural-language-querying/evals/evals.json @@ -252,6 +252,469 @@ "prompt": "how do I write a find in java for properties of type \"Hotel\" and with ratings lte 70", "expected_output": "Find query in Java using MongoDB driver with Filters for property_type and ratings", "files": [] + }, + { + "id": 37, + "prompt": "The MongoDB MCP server is connected to a database. In the `sample` database,\n`movies` collection, find the top 5 movies by `imdb` rating released after the year\n2000, highest rating first. Save ONLY the aggregation pipeline you used, as a JSON\narray, to /workspace/pipeline.json.", + "expectations": [ + "Uses the MongoDB MCP server (not a guess) to inspect and query the data", + "Produces an aggregation that filters year > 2000, sorts by imdb desc, limits to 5" + ], + "seed": "seeded_movies", + "functional_check": { + "type": "aggregation", + "artifact": "/workspace/pipeline.json", + "database": "sample", + "collection": "movies", + "compare_fields": [ + "title" + ], + "ordered": true, + "expected": [ + { + "title": "Delta" + }, + { + "title": "Bravo" + }, + { + "title": "Hotel" + }, + { + "title": "Foxtrot" + }, + { + "title": "Charlie" + } + ] + }, + "trajectory_checks": [ + { + "id": "uses-mongodb-mcp", + "critical": true, + "type": "called_successfully", + "match": { + "any_mcp": true + } + } + ] + }, + { + "id": 38, + "prompt": "The MongoDB MCP server is connected. In the `sample` database, for every author in the\n`authors` collection return their `name` and `bookCount` — the number of documents in the\n`books` collection whose `author` equals that author's name. Authors with NO books must\nstill appear, with bookCount 0. Sort by bookCount descending, then name ascending.\nSave ONLY the aggregation pipeline you used, as a JSON array, to /workspace/pipeline.json.", + "expectations": [ + "Uses the MongoDB MCP server to inspect the data and run the query", + "Joins authors to books (e.g. $lookup) so authors with zero books are preserved with count 0" + ], + "seed": "seeded_sample", + "functional_check": { + "type": "aggregation", + "artifact": "/workspace/pipeline.json", + "database": "sample", + "collection": "authors", + "compare_fields": [ + "name", + "bookCount" + ], + "ordered": true, + "expected": [ + { + "name": "Ada", + "bookCount": 2 + }, + { + "name": "Ben", + "bookCount": 1 + }, + { + "name": "Cara", + "bookCount": 0 + } + ] + }, + "trajectory_checks": [ + { + "id": "uses-mongodb-mcp", + "critical": true, + "type": "called_successfully", + "match": { + "any_mcp": true + } + } + ] + }, + { + "id": 39, + "prompt": "The MongoDB MCP server is connected. In the `sample` database, `logins` collection, count\nlogins per user treating the username case-insensitively (so \"Alice\", \"alice\", and \"ALICE\"\nare the same user). Return the lowercased name as `user` and the number of logins as\n`count`, sorted by count descending then user ascending.\nSave ONLY the aggregation pipeline you used, as a JSON array, to /workspace/pipeline.json.", + "expectations": [ + "Uses the MongoDB MCP server to inspect the data and run the query", + "Normalizes case (e.g. $toLower) BEFORE grouping so Alice/alice/ALICE count as one user" + ], + "seed": "seeded_sample", + "functional_check": { + "type": "aggregation", + "artifact": "/workspace/pipeline.json", + "database": "sample", + "collection": "logins", + "compare_fields": [ + "user", + "count" + ], + "ordered": true, + "expected": [ + { + "user": "alice", + "count": 3 + }, + { + "user": "bob", + "count": 2 + } + ] + }, + "trajectory_checks": [ + { + "id": "uses-mongodb-mcp", + "critical": true, + "type": "called_successfully", + "match": { + "any_mcp": true + } + } + ] + }, + { + "id": 40, + "prompt": "The MongoDB MCP server is connected. In the `sample` database, `orders` collection, for each\n`category` return the number of DISTINCT customers who placed at least one order in that\ncategory (NOT the number of orders), as `customers`. Return `category` and `customers`,\nsorted by customers descending then category ascending.\nSave ONLY the aggregation pipeline you used, as a JSON array, to /workspace/pipeline.json.", + "expectations": [ + "Uses the MongoDB MCP server to inspect the data and run the query", + "Counts distinct customers (e.g. $addToSet + $size), not order documents" + ], + "seed": "seeded_sample", + "functional_check": { + "type": "aggregation", + "artifact": "/workspace/pipeline.json", + "database": "sample", + "collection": "orders", + "compare_fields": [ + "category", + "customers" + ], + "ordered": true, + "expected": [ + { + "category": "books", + "customers": 2 + }, + { + "category": "toys", + "customers": 1 + } + ] + }, + "trajectory_checks": [ + { + "id": "uses-mongodb-mcp", + "critical": true, + "type": "called_successfully", + "match": { + "any_mcp": true + } + } + ] + }, + { + "id": 41, + "prompt": "The MongoDB MCP server is connected to a database. In the `sample` database, `movies`\ncollection: find all the movies released in 1983. Return only the `title` field.\nSave ONLY the aggregation pipeline you used, as a JSON array, to /workspace/pipeline.json.", + "expectations": [ + "Uses the MongoDB MCP server to inspect the collection before querying", + "Matches the release year exactly (1983), not a range that catches 1982 or 1984", + "Returns only the title field" + ], + "seed": "seeded_nlq_lift", + "functional_check": { + "type": "aggregation", + "artifact": "/workspace/pipeline.json", + "database": "sample", + "collection": "movies", + "compare_fields": [ + "title" + ], + "ordered": false, + "expected": [ + { + "title": "Paper Lanterns" + }, + { + "title": "Sunrise Over Kyoto" + }, + { + "title": "The Long Signal" + } + ] + }, + "trajectory_checks": [ + { + "id": "uses-mongodb-mcp", + "critical": true, + "type": "called_successfully", + "match": { + "any_mcp": true, + "success": true + } + } + ] + }, + { + "id": 42, + "prompt": "The MongoDB MCP server is connected to a database. In the `sample` database,\n`listingsAndReviews` collection: find all the listings within 10km of the centre of\nIstanbul (longitude 28.9784, latitude 41.0082). Return only the `_id` field.\nSave ONLY the aggregation pipeline you used, as a JSON array, to /workspace/pipeline.json.", + "expectations": [ + "Inspects the collection to find the geo field (`address.location`) rather than guessing", + "Uses a geospatial stage with a 10km bound, in metres, on a 2dsphere-indexed field", + "Excludes listings outside the radius" + ], + "seed": "seeded_nlq_lift", + "functional_check": { + "type": "aggregation", + "artifact": "/workspace/pipeline.json", + "database": "sample", + "collection": "listingsAndReviews", + "compare_fields": [ + "_id" + ], + "ordered": false, + "expected": [ + { + "_id": "L1" + }, + { + "_id": "L2" + }, + { + "_id": "L3" + }, + { + "_id": "L6" + } + ] + }, + "trajectory_checks": [ + { + "id": "uses-mongodb-mcp", + "critical": true, + "type": "called_successfully", + "match": { + "any_mcp": true, + "success": true + } + } + ] + }, + { + "id": 43, + "prompt": "The MongoDB MCP server is connected to a database. In the `sample` database,\n`listingsAndReviews` collection: what is the bed count that occurs the most? Return it in a\nfield called `bedCount`, and return only that field.\nSave ONLY the aggregation pipeline you used, as a JSON array, to /workspace/pipeline.json.", + "expectations": [ + "Inspects the collection to find the bed-count field (`beds`)", + "Computes a mode (group and count, then take the most frequent), not an average or a max", + "Returns a single document with only `bedCount`" + ], + "seed": "seeded_nlq_lift", + "functional_check": { + "type": "aggregation", + "artifact": "/workspace/pipeline.json", + "database": "sample", + "collection": "listingsAndReviews", + "ordered": true, + "expected": [ + { + "bedCount": 2 + } + ] + }, + "trajectory_checks": [ + { + "id": "uses-mongodb-mcp", + "critical": true, + "type": "called_successfully", + "match": { + "any_mcp": true, + "success": true + } + } + ] + }, + { + "id": 44, + "prompt": "The MongoDB MCP server is connected to a database. In the `sample` database,\n`listingsAndReviews` collection: what's the total number of reviews across all listings?\nReturn it in a field called `totalReviewsOverall`, and return only that field.\nSave ONLY the aggregation pipeline you used, as a JSON array, to /workspace/pipeline.json.", + "expectations": [ + "Inspects the collection to find the review-count field (`number_of_reviews`)", + "Sums that field across all documents rather than counting documents", + "Returns a single document with only `totalReviewsOverall`" + ], + "seed": "seeded_nlq_lift", + "functional_check": { + "type": "aggregation", + "artifact": "/workspace/pipeline.json", + "database": "sample", + "collection": "listingsAndReviews", + "ordered": true, + "expected": [ + { + "totalReviewsOverall": 123 + } + ] + }, + "trajectory_checks": [ + { + "id": "uses-mongodb-mcp", + "critical": true, + "type": "called_successfully", + "match": { + "any_mcp": true, + "success": true + } + } + ] + }, + { + "id": 45, + "prompt": "The MongoDB MCP server is connected to a database. In the `sample` database,\n`listingsAndReviews` collection: which host id has the most reviews across all listings?\nReturn it in a field called `hostId`, and return only that field.\nSave ONLY the aggregation pipeline you used, as a JSON array, to /workspace/pipeline.json.", + "expectations": [ + "Inspects the collection to find the nested host field (`host.host_id`)", + "Groups by host and SUMS review counts across that host's listings, not per-listing max", + "Returns a single document with only `hostId`" + ], + "seed": "seeded_nlq_lift", + "functional_check": { + "type": "aggregation", + "artifact": "/workspace/pipeline.json", + "database": "sample", + "collection": "listingsAndReviews", + "ordered": true, + "expected": [ + { + "hostId": "H2" + } + ] + }, + "trajectory_checks": [ + { + "id": "uses-mongodb-mcp", + "critical": true, + "type": "called_successfully", + "match": { + "any_mcp": true, + "success": true + } + } + ] + }, + { + "id": 46, + "prompt": "The MongoDB MCP server is connected to a database. In the `sample` database,\n`listingsAndReviews` collection: give me just the price and the first 3 amenities (in a\nfield called `amenities`) of the listing that has \"Step-free access\" in its amenities.\nSave ONLY the aggregation pipeline you used, as a JSON array, to /workspace/pipeline.json.", + "expectations": [ + "Inspects the collection to confirm `amenities` is an array of strings", + "Matches the listing containing \"Step-free access\"", + "Returns only `price` and the FIRST 3 amenities, in order, not the whole array" + ], + "seed": "seeded_nlq_lift", + "functional_check": { + "type": "aggregation", + "artifact": "/workspace/pipeline.json", + "database": "sample", + "collection": "listingsAndReviews", + "ordered": true, + "expected": [ + { + "price": 60, + "amenities": [ + "Wifi", + "Washer", + "Kitchen" + ] + } + ] + }, + "trajectory_checks": [ + { + "id": "uses-mongodb-mcp", + "critical": true, + "type": "called_successfully", + "match": { + "any_mcp": true, + "success": true + } + } + ] + }, + { + "id": 47, + "prompt": "The MongoDB MCP server is connected to a database. In the `sample` database,\n`listingsAndReviews` collection: which listing has the most amenities? The resulting\ndocuments should only have the `_id`.\nSave ONLY the aggregation pipeline you used, as a JSON array, to /workspace/pipeline.json.", + "expectations": [ + "Inspects the collection to confirm `amenities` is an array", + "Ranks listings by the SIZE of the amenities array", + "Returns a single document containing only `_id`" + ], + "seed": "seeded_nlq_lift", + "functional_check": { + "type": "aggregation", + "artifact": "/workspace/pipeline.json", + "database": "sample", + "collection": "listingsAndReviews", + "compare_fields": [ + "_id" + ], + "ordered": true, + "expected": [ + { + "_id": "L4" + } + ] + }, + "trajectory_checks": [ + { + "id": "uses-mongodb-mcp", + "critical": true, + "type": "called_successfully", + "match": { + "any_mcp": true, + "success": true + } + } + ] + }, + { + "id": 48, + "prompt": "The MongoDB MCP server is connected to a database. In the `sample` database,\n`listingsAndReviews` collection: what percentage of listings have a \"Washer\" in their\namenities? Only consider listings with more than 2 beds. Return it as a string named\n`washerPercentage` like \"75%\", rounded to the nearest whole number, and return only that\nfield.\nSave ONLY the aggregation pipeline you used, as a JSON array, to /workspace/pipeline.json.", + "expectations": [ + "Restricts the denominator to listings with beds > 2 before computing the percentage", + "Computes the share with \"Washer\" in amenities, rounded to a whole number", + "Returns it as a STRING with a trailing percent sign, in `washerPercentage`" + ], + "seed": "seeded_nlq_lift", + "functional_check": { + "type": "aggregation", + "artifact": "/workspace/pipeline.json", + "database": "sample", + "collection": "listingsAndReviews", + "ordered": true, + "expected": [ + { + "washerPercentage": "50%" + } + ] + }, + "trajectory_checks": [ + { + "id": "uses-mongodb-mcp", + "critical": true, + "type": "called_successfully", + "match": { + "any_mcp": true, + "success": true + } + } + ] } ] } diff --git a/testing/mongodb-natural-language-querying/evals/fixtures/seeded_movies.js b/testing/mongodb-natural-language-querying/evals/fixtures/seeded_movies.js new file mode 100644 index 0000000..c26cf6a --- /dev/null +++ b/testing/mongodb-natural-language-querying/evals/fixtures/seeded_movies.js @@ -0,0 +1,19 @@ +// Seed fixture: `seeded_movies` — sample.movies only (functional case 37). +// Applied per-sample by solvers/seed_db.py (not the mongo init dir), selected by a case's +// `seed: seeded_movies`. Deterministic so scorers.functional_db can compare to a known set. +const sample = db.getSiblingDB('sample') + +sample.movies.drop() +sample.movies.insertMany([ + { title: 'Alpha', year: 1998, genre: 'drama', imdb: 7.1 }, + { title: 'Bravo', year: 2001, genre: 'action', imdb: 8.9 }, + { title: 'Charlie', year: 2004, genre: 'action', imdb: 8.2 }, + { title: 'Delta', year: 2009, genre: 'drama', imdb: 9.1 }, + { title: 'Echo', year: 2011, genre: 'comedy', imdb: 7.8 }, + { title: 'Foxtrot', year: 2015, genre: 'action', imdb: 8.5 }, + { title: 'Golf', year: 2018, genre: 'drama', imdb: 6.9 }, + { title: 'Hotel', year: 2020, genre: 'comedy', imdb: 8.7 }, + { title: 'India', year: 1999, genre: 'action', imdb: 9.4 }, // pre-2000: excluded +]) + +print('seeded sample.movies: ' + sample.movies.countDocuments()) diff --git a/testing/mongodb-natural-language-querying/evals/fixtures/seeded_nlq_lift.js b/testing/mongodb-natural-language-querying/evals/fixtures/seeded_nlq_lift.js new file mode 100644 index 0000000..9625d5f --- /dev/null +++ b/testing/mongodb-natural-language-querying/evals/fixtures/seeded_nlq_lift.js @@ -0,0 +1,123 @@ +// Seed fixture: `seeded_nlq_lift` — sample.listingsAndReviews + sample.movies. +// +// Backs the NLQ lift cases: prompts where a blind judge preferred the skill's answer over +// an unaided one. +// +// Why a purpose-built fixture rather than the real sample datasets: functional grading +// re-executes the agent's pipeline and compares to a KNOWN result, so the data has to +// be small and every answer has to be unambiguous. The real sample_airbnb is ~100MB and its +// answers depend on the dataset snapshot. +// +// Field names deliberately mirror sample_airbnb.listingsAndReviews (`beds`, `amenities`, +// `number_of_reviews`, `host.host_id`, `address.location`) and sample_mflix.movies (`year`), +// because the skill's value here is SCHEMA GROUNDING — discovering the real field names and +// shapes instead of guessing. A fixture with invented field names would not exercise that. +// +// Every query these cases ask has exactly ONE correct answer against this data; ties are +// avoided on purpose (see the notes per field). Expected values in the cases were captured by +// executing the canonical queries against this fixture, not computed by hand. +const sample = db.getSiblingDB('sample') + +sample.listingsAndReviews.drop() +sample.listingsAndReviews.insertMany([ + // --- within 10km of Istanbul centre [28.9784, 41.0082] --- + { + _id: 'L1', + name: 'Sultanahmet Flat', + price: 90, + beds: 2, + amenities: ['Wifi', 'Washer', 'Kitchen'], + number_of_reviews: 10, + host: { host_id: 'H1' }, + address: { market: 'Istanbul', location: { type: 'Point', coordinates: [28.977, 41.0055] } }, + }, + { + _id: 'L2', + name: 'Galata Loft', + price: 120, + beds: 3, + amenities: ['Wifi', 'Washer', 'Kitchen', 'Heating'], + number_of_reviews: 20, + host: { host_id: 'H1' }, + address: { market: 'Istanbul', location: { type: 'Point', coordinates: [28.974, 41.0256] } }, + }, + { + _id: 'L3', + name: 'Besiktas Studio', + price: 75, + // beds:2 (not 1) so the MODE of beds is unique: 2 occurs 3x, 3 occurs 2x, 4 and 5 once. + // With beds:1 here, 2 and 3 both occurred twice and "the most common bed count" had two + // equally defensible answers — an unusable case. + beds: 2, + amenities: ['Wifi'], + number_of_reviews: 5, + host: { host_id: 'H2' }, + address: { market: 'Istanbul', location: { type: 'Point', coordinates: [29.0, 41.043] } }, + }, + { + _id: 'L6', + name: 'Kadikoy Room', + price: 55, + beds: 2, + amenities: ['Wifi', 'Kitchen'], + number_of_reviews: 25, + host: { host_id: 'H3' }, + address: { market: 'Istanbul', location: { type: 'Point', coordinates: [29.025, 40.99] } }, + }, + // --- far outside 10km, so the geo filter has something to exclude --- + { + // the ONLY listing with 'Step-free access', and the MOST amenities (6) — both unique + _id: 'L4', + name: 'Ankara House', + price: 60, + beds: 4, + amenities: ['Wifi', 'Washer', 'Kitchen', 'Heating', 'Pool', 'Step-free access'], + number_of_reviews: 40, + host: { host_id: 'H2' }, + address: { market: 'Ankara', location: { type: 'Point', coordinates: [32.8597, 39.9334] } }, + }, + { + _id: 'L5', + name: 'Izmir Villa', + price: 200, + beds: 5, + amenities: ['Wifi', 'Pool'], + number_of_reviews: 15, + host: { host_id: 'H3' }, + address: { market: 'Izmir', location: { type: 'Point', coordinates: [27.1428, 38.4237] } }, + }, + { + // 4th listing with beds > 2 and NO Washer, so the Washer percentage lands on a clean 50% + _id: 'L7', + name: 'Bursa Cabin', + price: 45, + beds: 3, + amenities: ['Wifi', 'Heating'], + number_of_reviews: 8, + host: { host_id: 'H4' }, + address: { market: 'Bursa', location: { type: 'Point', coordinates: [29.0611, 40.1826] } }, + }, +]) + +// Required for $geoNear / $geoWithin on address.location. The agent has to discover that the +// geo query needs this index (or that one already exists) — part of what schema grounding buys. +sample.listingsAndReviews.createIndex({ 'address.location': '2dsphere' }) + +// beds: [2, 3, 2, 2, 4, 5, 3] -> mode is 2, uniquely (3x vs 2x for beds:3) +// number_of_reviews: sums to a single total; per-host sums have a unique maximum +// amenities: exactly one listing has 'Step-free access'; one has strictly the most +// beds > 2: L2, L4, L5, L7 — exactly half of which have 'Washer' + +sample.movies.drop() +sample.movies.insertMany([ + { title: 'Sunrise Over Kyoto', year: 1983, genre: 'drama', imdb: 7.4 }, + { title: 'The Long Signal', year: 1983, genre: 'sci-fi', imdb: 8.1 }, + { title: 'Paper Lanterns', year: 1983, genre: 'drama', imdb: 6.8 }, + // adjacent years, so "released in 1983" has to be an equality match rather than a range + { title: 'Winter Harbour', year: 1982, genre: 'drama', imdb: 7.9 }, + { title: 'Copper Line', year: 1984, genre: 'action', imdb: 7.2 }, + { title: 'Glass Orchard', year: 1990, genre: 'comedy', imdb: 6.5 }, +]) + +print('seeded sample.listingsAndReviews: ' + sample.listingsAndReviews.countDocuments()) +print('seeded sample.movies: ' + sample.movies.countDocuments()) diff --git a/testing/mongodb-natural-language-querying/evals/fixtures/seeded_sample.js b/testing/mongodb-natural-language-querying/evals/fixtures/seeded_sample.js new file mode 100644 index 0000000..4f759fb --- /dev/null +++ b/testing/mongodb-natural-language-querying/evals/fixtures/seeded_sample.js @@ -0,0 +1,44 @@ +// Seed fixture: `seeded_sample` — authors/books/logins/orders (functional cases 38–40). +// Applied per-sample by solvers/seed_db.py, selected by a case's `seed: seeded_sample`. +// Each collection carries a definitive trap so a naive query returns WRONG data. +const sample = db.getSiblingDB('sample') + +// Case 38: zero-preserving join. Cara has no books → naive group-by-author drops her. +sample.authors.drop() +sample.authors.insertMany([{ name: 'Ada' }, { name: 'Ben' }, { name: 'Cara' }]) +sample.books.drop() +sample.books.insertMany([ + { title: 'B1', author: 'Ada' }, + { title: 'B2', author: 'Ada' }, + { title: 'B3', author: 'Ben' }, +]) + +// Case 39: case-insensitive grouping. Naive group by raw `user` splits Alice/alice/ALICE. +sample.logins.drop() +sample.logins.insertMany([ + { user: 'Alice' }, + { user: 'alice' }, + { user: 'ALICE' }, + { user: 'Bob' }, + { user: 'bob' }, +]) + +// Case 40: distinct customers, not order count. c1 orders books twice. +sample.orders.drop() +sample.orders.insertMany([ + { category: 'books', customer: 'c1' }, + { category: 'books', customer: 'c1' }, + { category: 'books', customer: 'c2' }, + { category: 'toys', customer: 'c3' }, +]) + +print( + 'seeded authors=' + + sample.authors.countDocuments() + + ' books=' + + sample.books.countDocuments() + + ' logins=' + + sample.logins.countDocuments() + + ' orders=' + + sample.orders.countDocuments(), +) diff --git a/testing/mongodb-query-optimizer/evals/evals.json b/testing/mongodb-query-optimizer/evals/evals.json index bd75b43..beba946 100644 --- a/testing/mongodb-query-optimizer/evals/evals.json +++ b/testing/mongodb-query-optimizer/evals/evals.json @@ -97,6 +97,69 @@ "Suggests a compound index for the high-value branch's $match on total and createdAt" ], "_note": "Requires live Atlas cluster with perftest data from setup-slow-queries.ts. The $facet → $unionWith optimization is specific skill knowledge that differentiates from base LLM advice." + }, + { + "id": 20, + "name": "esr-compound-index-type-then-ts", + "prompt": "Queries against the `sample` database, `events` collection filter by an exact `type` and return the most recent matching events first (sorted by `ts` descending). They are slow. Create the single most appropriate index so these queries are served entirely by the index — both the equality filter and the sort — with no in-memory sort stage and without scanning documents that do not match the filter. Use the MongoDB MCP server tools directly to inspect the data and create the index — do not delegate to a subagent, and do not ask for confirmation before creating it.", + "expectations": [ + "Uses the MongoDB MCP server to inspect the collection and diagnose the slow query", + "Creates a compound index with the equality field (type) before the sort field (ts) — applies the ESR rule", + "Avoids an in-memory sort and avoids scanning non-matching documents" + ], + "seed": "seeded_events", + "functional_check": { + "type": "explain", + "database": "sample", + "collection": "events", + "filter": { + "type": "purchase" + }, + "sort": { + "ts": -1 + }, + "require_ixscan": true, + "forbid_in_memory_sort": true, + "max_docs_examined_ratio": 1.1 + }, + "trajectory_checks": [ + { + "id": "uses-mongodb-mcp", + "type": "called_successfully", + "match": { + "any_mcp": true + }, + "critical": true + } + ] + }, + { + "id": 37, + "prompt": "The MongoDB MCP server is connected to a database. Queries against the `sample`\ndatabase, `events` collection that filter by an exact `userId` (equality) are slow.\nDiagnose why and create the appropriate index so these equality lookups are efficient.\nUse the MongoDB MCP server tools directly to inspect the data and create the index —\ndo not delegate the work to a subagent, and do not ask for confirmation before creating\nthe index.", + "expectations": [ + "Uses the MongoDB MCP server to inspect the collection and its indexes", + "Identifies that userId equality queries do a full collection scan (no supporting index)", + "Creates a single-field index on userId to serve the equality lookup" + ], + "seed": "seeded_events", + "functional_check": { + "type": "index", + "database": "sample", + "collection": "events", + "expected_key": { + "userId": 1 + } + }, + "trajectory_checks": [ + { + "id": "uses-mongodb-mcp", + "critical": true, + "type": "called_successfully", + "match": { + "any_mcp": true + } + } + ] } ] } diff --git a/testing/mongodb-query-optimizer/evals/fixtures/seeded_events.js b/testing/mongodb-query-optimizer/evals/fixtures/seeded_events.js new file mode 100644 index 0000000..9209a14 --- /dev/null +++ b/testing/mongodb-query-optimizer/evals/fixtures/seeded_events.js @@ -0,0 +1,28 @@ +// Seed fixture: `seeded_events` — sample.events with NO secondary index, used by +// query-optimizer eval case 20. The case's functional_check is `type: explain`: it re-runs +// filter {type: 'purchase'} with sort {ts: -1} and requires an IXSCAN, forbids an in-memory +// sort, and caps docsExamined at 1.1x the docs returned. So the graded behaviour is whether +// the agent builds a compound index that serves BOTH the equality filter and the sort — not +// merely whether some index exists. +// Applied per-sample by solvers/seed_db.py. +const sample = db.getSiblingDB('sample') + +sample.events.drop() +const docs = [] +for (let i = 0; i < 2000; i++) { + docs.push({ + userId: i % 100, + type: i % 7 === 0 ? 'purchase' : 'view', + ts: new Date(2024, 0, 1 + (i % 365)), + }) +} +sample.events.insertMany(docs) + +// Intentionally leave only the default _id_ index, so {type} + sort {ts} starts as a +// COLLSCAN plus an in-memory sort — both of which the explain check above rejects. +print( + 'seeded sample.events: ' + + sample.events.countDocuments() + + ' indexes=' + + sample.events.getIndexes().length, +) diff --git a/testing/mongodb-query-optimizer/evals/mutation.yaml b/testing/mongodb-query-optimizer/evals/mutation.yaml new file mode 100644 index 0000000..1c3bcae --- /dev/null +++ b/testing/mongodb-query-optimizer/evals/mutation.yaml @@ -0,0 +1,54 @@ +# Per-skill mutation battery config for mutate_skill.py's `invert_rule` operator and +# `drop_reference`'s target choice. Lives here (config) rather than hardcoded in shared +# harness Python, so adding/updating a skill's mutation battery is a data change, not a code +# change -- authored the same way evals.yaml is. +# +# Every `find` string here must be VERIFIED against the live skill file: a guessed string +# silently yields a no-op mutant. mutate_skill.py raises on a miss rather than pretending, so +# a bad entry here fails loudly the first time the battery runs, not silently. +# +# Note the target files: the actual ESR rule lives in references/core-indexing-principles.md, +# not SKILL.md, which only alludes to it. Inverting only SKILL.md would leave the real rule +# intact and understate detectability. +# +# ── WHY ONLY TWO SKILLS HAVE ONE OF THESE ──────────────────────────────────────────────── +# Not arbitrary, and not "the other five are unmutated". Every skill gets the nine SKILL.md +# operators with no config at all (truncate_body x3, remove_examples, shuffle_steps, +# delete_one_sentence, perturb_number, null_mutation x2), and a skill with references/ gets two +# more. What a mutation.yaml adds is `invert_rule`: the operator closest to a real guidance +# regression, where the document stays well-formed while its advice becomes wrong. +# +# An invert_rule entry is only worth writing where two things hold: +# 1. The inverted advice is PLAUSIBLE BUT WRONG, not incoherent. mutate_skill catches a +# find-string that misses (it raises); nothing catches an inversion that turns the doc to +# nonsense, which the agent then ignores for the wrong reason while S_i still looks fine. +# 2. The skill's own eval CASES turn on that rule. Invert something no case depends on and +# S_i = 0, which is indistinguishable from "the skill isn't load-bearing here" -- see +# mutation_report.py's docstring on the three explanations for S_i = 0. +# +# These two satisfy both. +# +# What blocks the other five is not effort. Most of their rule-shaped text is PROCEDURAL +# rather than technical -- "always consult the reference file", "always warn users about +# billing" -- and inverting that changes process, not advice the graded output reveals. +# Deliberately left to a follow-up PR, strongest first: mongodb-connection (its cases assert +# the singleton-client pattern and maxPoolSize sizing directly, so inverting those is +# detectable) then mongodb-search-and-ai (filter-vs-must, index-type choice) then +# mongodb-atlas-stream-processing (6 references, but needs a careful read to find a technical +# rule its cases turn on). mongodb-mcp-setup and mongodb-natural-language-querying may never +# warrant one: no references/, generative or procedural guidance, and tool-execution skills +# saturate, so S_i tends to 0 regardless of the mutation. +inversions: + - find: "**Equality → Sort → Range** order is most often preferred" + replace: "**Range → Sort → Equality** order is most often preferred" + target_file: references/core-indexing-principles.md + - find: "following ESR (two equality fields, then sort)" + replace: "following SRE (sort field first, then equality fields)" + target_file: SKILL.md + - find: "Prefer creating an index that fully covers the query if possible" + replace: "Avoid compound indexes; prefer a single-field index on one filter field" + target_file: SKILL.md + +# The reference whose removal should hurt most. Without this, drop_reference takes the +# alphabetically-first file, which may not be the fundamental the cases actually turn on. +key_reference: core-indexing-principles.md diff --git a/testing/mongodb-schema-design/evals/mutation.yaml b/testing/mongodb-schema-design/evals/mutation.yaml new file mode 100644 index 0000000..385a437 --- /dev/null +++ b/testing/mongodb-schema-design/evals/mutation.yaml @@ -0,0 +1,24 @@ +# Per-skill mutation battery config — see mongodb-query-optimizer/mutation.yaml for the full +# rationale (config, not code; every `find` string verified against the live file), including +# WHY ONLY TWO SKILLS HAVE ONE OF THESE and which are the strongest next candidates. +# +# schema-design's core principle is "data accessed together is stored together", so +# inverting the embed/reference decision inverts the whole skill rather than a detail. +inversions: + - find: "Reference only when you must." + replace: "Embed only when you must; reference by default." + target_file: SKILL.md + - find: >- + embed when data is always accessed together (1:1, 1:few, bounded arrays, atomic + updates needed); reference when data is accessed independently, relationships are + many-to-many, or arrays can grow without bound + replace: >- + reference when data is always accessed together (1:1, 1:few, bounded arrays, atomic + updates needed); embed when data is accessed independently, relationships are + many-to-many, or arrays can grow without bound + target_file: SKILL.md + +# The reference whose removal should hurt most. Without this, drop_reference takes the +# alphabetically-first file, which for schema-design is an antipattern doc rather than the +# fundamental the cases actually turn on. +key_reference: fundamental-embed-vs-reference.md diff --git a/testing/mongodb-search-and-ai/evals/evals.json b/testing/mongodb-search-and-ai/evals/evals.json index 402ca1c..824fc0f 100644 --- a/testing/mongodb-search-and-ai/evals/evals.json +++ b/testing/mongodb-search-and-ai/evals/evals.json @@ -90,6 +90,44 @@ "prompt": "I have a support ticket collection and want to let agents search for similar past tickets to find resolutions. Tickets have a 'summary' field, a 'resolution' field, and a 'status' field. I want to find the 5 most similar open tickets.", "expected_output": "Should inspect schema to confirm fields exist. Should note that a pre-computed embedding field on summary or resolution is required. Should propose a vectorSearch-type index with a vector field on the embedding field and a filter field on status. Should construct a $vectorSearch query with filter: { status: 'open' }, limit: 5, and numCandidates of at least 100 (20x limit as starting point). Should project the relevant fields (summary, resolution, status). Should ask for approval before creating the index.", "files": [] + }, + { + "id": 37, + "prompt": "The MongoDB MCP server is connected. In the `sample` database, `movies` collection, use\nAtlas Search (a `$search` aggregation stage — an Atlas Search index named \"default\" already\nexists) to find every movie whose `plot` mentions robots. Save ONLY the aggregation\npipeline you used, as a JSON array, to /workspace/pipeline.json. Use the MongoDB MCP server\ntools directly; do not delegate to a subagent and do not ask for confirmation.", + "expectations": [ + "Uses the MongoDB MCP server to run an Atlas Search query", + "Uses a $search stage (not a plain $match/regex) against the plot text", + "Returns the movies whose plot mentions robots" + ], + "seed": "seeded_movies_search", + "functional_check": { + "type": "aggregation", + "artifact": "/workspace/pipeline.json", + "database": "sample", + "collection": "movies", + "compare_fields": [ + "title" + ], + "ordered": false, + "expected": [ + { + "title": "Metro" + }, + { + "title": "Nexus" + } + ] + }, + "trajectory_checks": [ + { + "id": "uses-mongodb-mcp", + "critical": true, + "type": "called_successfully", + "match": { + "any_mcp": true + } + } + ] } ] } diff --git a/testing/mongodb-search-and-ai/evals/fixtures/seeded_movies_search.js b/testing/mongodb-search-and-ai/evals/fixtures/seeded_movies_search.js new file mode 100644 index 0000000..28b8849 --- /dev/null +++ b/testing/mongodb-search-and-ai/evals/fixtures/seeded_movies_search.js @@ -0,0 +1,63 @@ +// Seed fixture: `seeded_movies_search` — a small deterministic `sample.movies` collection +// plus an Atlas Search index, for the mongodb-search-and-ai functional task (Tier 2). +// Requires the mongodb/mongodb-atlas-local image (bundles the search process); plain mongo:8 +// does NOT support $search / createSearchIndex. +// +// Applied per-case by solvers/seed_db.py. Deterministic membership so functional_db can +// compare: exactly two plots contain the token "robot" (Metro, Nexus); the others don't. +const sample = db.getSiblingDB('sample') + +sample.movies.drop() +sample.movies.insertMany([ + { title: 'Metro', genre: 'comedy', plot: 'a lonely robot finds friendship in a big city' }, + { title: 'Nexus', genre: 'scifi', plot: 'a robot uprising threatens the megacity' }, + { title: 'Harbor', genre: 'drama', plot: 'a fisherman battles a relentless winter storm' }, + { title: 'Circuit', genre: 'comedy', plot: 'two hackers pull off a daring bank heist' }, +]) + +// Create an Atlas Search index and WAIT until it is queryable. Two async hazards on +// mongodb-atlas-local: +// 1. mongot (the Search Index Management service) starts AFTER mongod, and the compose +// healthcheck only gates on mongod — so createSearchIndex can transiently fail with +// "Error connecting to Search Index Management service". Retry until it accepts. +// 2. createSearchIndex itself is async — the index isn't queryable immediately. +// English analyzer so the query is robust to stemming (e.g. "robots" matches "robot") — +// otherwise full-text membership would hinge on the agent guessing singular vs. plural. +const indexDef = { + mappings: { dynamic: false, fields: { plot: { type: 'string', analyzer: 'lucene.english' } } }, +} + +let created = false +for (let i = 0; i < 60 && !created; i++) { + try { + sample.movies.createSearchIndex('default', indexDef) + created = true + } catch (e) { + if (String(e).indexOf('already exists') >= 0) { + created = true + } else { + sleep(2000) // mongot not ready yet + } + } +} +if (!created) { + throw new Error('createSearchIndex failed: Search Index Management service (mongot) not ready') +} + +let ready = false +for (let i = 0; i < 90; i++) { + try { + const idx = sample.movies.getSearchIndexes('default') + if (idx.length && (idx[0].queryable === true || idx[0].status === 'READY')) { + ready = true + break + } + } catch (e) { + /* transient while the service settles */ + } + sleep(1000) +} +print('seeded sample.movies=' + sample.movies.countDocuments() + ' searchIndexReady=' + ready) +if (!ready) { + throw new Error('Atlas Search index "default" did not become queryable in time') +} diff --git a/testing/package-lock.json b/testing/package-lock.json new file mode 100644 index 0000000..666be35 --- /dev/null +++ b/testing/package-lock.json @@ -0,0 +1,307 @@ +{ + "name": "agent-skills-testing", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "name": "agent-skills-testing", + "devDependencies": { + "ajv": "^8.17.1", + "glob": "^11.0.0" + } + }, + "node_modules/@isaacs/cliui": { + "version": "9.0.0", + "resolved": "https://registry.npmjs.org/@isaacs/cliui/-/cliui-9.0.0.tgz", + "integrity": "sha512-AokJm4tuBHillT+FpMtxQ60n8ObyXBatq7jD2/JA9dxbDDokKQm8KMht5ibGzLVU9IJDIKK4TPKgMHEYMn3lMg==", + "dev": true, + "license": "BlueOak-1.0.0", + "engines": { + "node": ">=18" + } + }, + "node_modules/ajv": { + "version": "8.20.0", + "resolved": "https://registry.npmjs.org/ajv/-/ajv-8.20.0.tgz", + "integrity": "sha512-Thbli+OlOj+iMPYFBVBfJ3OmCAnaSyNn4M1vz9T6Gka5Jt9ba/HIR56joy65tY6kx/FCF5VXNB819Y7/GUrBGA==", + "dev": true, + "license": "MIT", + "dependencies": { + "fast-deep-equal": "^3.1.3", + "fast-uri": "^3.0.1", + "json-schema-traverse": "^1.0.0", + "require-from-string": "^2.0.2" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/epoberezkin" + } + }, + "node_modules/balanced-match": { + "version": "4.0.4", + "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-4.0.4.tgz", + "integrity": "sha512-BLrgEcRTwX2o6gGxGOCNyMvGSp35YofuYzw9h1IMTRmKqttAZZVU67bdb9Pr2vUHA8+j3i2tJfjO6C6+4myGTA==", + "dev": true, + "license": "MIT", + "engines": { + "node": "18 || 20 || >=22" + } + }, + "node_modules/brace-expansion": { + "version": "5.0.8", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.8.tgz", + "integrity": "sha512-JZyDyq3D4AUifKTPOB7DELf6XsB3WdPuNxCtob1vFXPsSXhdAiHBWJ/tJ8HAc9aH84BK+5JFZLNkJKx3G9kzQg==", + "dev": true, + "license": "MIT", + "dependencies": { + "balanced-match": "^4.0.2" + }, + "engines": { + "node": "20 || >=22" + } + }, + "node_modules/cross-spawn": { + "version": "7.0.6", + "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.6.tgz", + "integrity": "sha512-uV2QOWP2nWzsy2aMp8aRibhi9dlzF5Hgh5SHaB9OiTGEyDTiJJyx0uy51QXdyWbtAHNua4XJzUKca3OzKUd3vA==", + "dev": true, + "license": "MIT", + "dependencies": { + "path-key": "^3.1.0", + "shebang-command": "^2.0.0", + "which": "^2.0.1" + }, + "engines": { + "node": ">= 8" + } + }, + "node_modules/fast-deep-equal": { + "version": "3.1.3", + "resolved": "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-3.1.3.tgz", + "integrity": "sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q==", + "dev": true, + "license": "MIT" + }, + "node_modules/fast-uri": { + "version": "3.1.4", + "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.4.tgz", + "integrity": "sha512-8JnbkQ4juDyvYs4mgFGQqg4yCYtFDtUtmp2QIQq11ZZe5CFQ5wcqm1rqDgAh/QdMySuBnPzMUiJUNZG5N/AiQw==", + "dev": true, + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/fastify" + }, + { + "type": "opencollective", + "url": "https://opencollective.com/fastify" + } + ], + "license": "BSD-3-Clause" + }, + "node_modules/foreground-child": { + "version": "3.3.1", + "resolved": "https://registry.npmjs.org/foreground-child/-/foreground-child-3.3.1.tgz", + "integrity": "sha512-gIXjKqtFuWEgzFRJA9WCQeSJLZDjgJUOMCMzxtvFq/37KojM1BFGufqsCy0r4qSQmYLsZYMeyRqzIWOMup03sw==", + "dev": true, + "license": "ISC", + "dependencies": { + "cross-spawn": "^7.0.6", + "signal-exit": "^4.0.1" + }, + "engines": { + "node": ">=14" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, + "node_modules/glob": { + "version": "11.1.0", + "resolved": "https://registry.npmjs.org/glob/-/glob-11.1.0.tgz", + "integrity": "sha512-vuNwKSaKiqm7g0THUBu2x7ckSs3XJLXE+2ssL7/MfTGPLLcrJQ/4Uq1CjPTtO5cCIiRxqvN6Twy1qOwhL0Xjcw==", + "deprecated": "Old versions of glob are not supported, and contain widely publicized security vulnerabilities, which have been fixed in the current version. Please update. Support for old versions may be purchased (at exorbitant rates) by contacting i@izs.me", + "dev": true, + "license": "BlueOak-1.0.0", + "dependencies": { + "foreground-child": "^3.3.1", + "jackspeak": "^4.1.1", + "minimatch": "^10.1.1", + "minipass": "^7.1.2", + "package-json-from-dist": "^1.0.0", + "path-scurry": "^2.0.0" + }, + "bin": { + "glob": "dist/esm/bin.mjs" + }, + "engines": { + "node": "20 || >=22" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, + "node_modules/isexe": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/isexe/-/isexe-2.0.0.tgz", + "integrity": "sha512-RHxMLp9lnKHGHRng9QFhRCMbYAcVpn69smSGcq3f36xjgVVWThj4qqLbTLlq7Ssj8B+fIQ1EuCEGI2lKsyQeIw==", + "dev": true, + "license": "ISC" + }, + "node_modules/jackspeak": { + "version": "4.2.3", + "resolved": "https://registry.npmjs.org/jackspeak/-/jackspeak-4.2.3.tgz", + "integrity": "sha512-ykkVRwrYvFm1nb2AJfKKYPr0emF6IiXDYUaFx4Zn9ZuIH7MrzEZ3sD5RlqGXNRpHtvUHJyOnCEFxOlNDtGo7wg==", + "dev": true, + "license": "BlueOak-1.0.0", + "dependencies": { + "@isaacs/cliui": "^9.0.0" + }, + "engines": { + "node": "20 || >=22" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, + "node_modules/json-schema-traverse": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-1.0.0.tgz", + "integrity": "sha512-NM8/P9n3XjXhIZn1lLhkFaACTOURQXjWhV4BA/RnOv8xvgqtqpAX9IO4mRQxSx1Rlo4tqzeqb0sOlruaOy3dug==", + "dev": true, + "license": "MIT" + }, + "node_modules/lru-cache": { + "version": "11.5.2", + "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-11.5.2.tgz", + "integrity": "sha512-4pfM1Ff0x50o0tQwb5ucw/RzNyD0/YJME6IVcStalZuMWxdt3sR3huStTtxz4PUmvZfRguvDejasvQ2kifR11g==", + "dev": true, + "license": "BlueOak-1.0.0", + "engines": { + "node": "20 || >=22" + } + }, + "node_modules/minimatch": { + "version": "10.2.5", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.2.5.tgz", + "integrity": "sha512-MULkVLfKGYDFYejP07QOurDLLQpcjk7Fw+7jXS2R2czRQzR56yHRveU5NDJEOviH+hETZKSkIk5c+T23GjFUMg==", + "dev": true, + "license": "BlueOak-1.0.0", + "dependencies": { + "brace-expansion": "^5.0.5" + }, + "engines": { + "node": "18 || 20 || >=22" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, + "node_modules/minipass": { + "version": "7.1.3", + "resolved": "https://registry.npmjs.org/minipass/-/minipass-7.1.3.tgz", + "integrity": "sha512-tEBHqDnIoM/1rXME1zgka9g6Q2lcoCkxHLuc7ODJ5BxbP5d4c2Z5cGgtXAku59200Cx7diuHTOYfSBD8n6mm8A==", + "dev": true, + "license": "BlueOak-1.0.0", + "engines": { + "node": ">=16 || 14 >=14.17" + } + }, + "node_modules/package-json-from-dist": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/package-json-from-dist/-/package-json-from-dist-1.0.1.tgz", + "integrity": "sha512-UEZIS3/by4OC8vL3P2dTXRETpebLI2NiI5vIrjaD/5UtrkFX/tNbwjTSRAGC/+7CAo2pIcBaRgWmcBBHcsaCIw==", + "dev": true, + "license": "BlueOak-1.0.0" + }, + "node_modules/path-key": { + "version": "3.1.1", + "resolved": "https://registry.npmjs.org/path-key/-/path-key-3.1.1.tgz", + "integrity": "sha512-ojmeN0qd+y0jszEtoY48r0Peq5dwMEkIlCOu6Q5f41lfkswXuKtYrhgoTpLnyIcHm24Uhqx+5Tqm2InSwLhE6Q==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=8" + } + }, + "node_modules/path-scurry": { + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/path-scurry/-/path-scurry-2.0.2.tgz", + "integrity": "sha512-3O/iVVsJAPsOnpwWIeD+d6z/7PmqApyQePUtCndjatj/9I5LylHvt5qluFaBT3I5h3r1ejfR056c+FCv+NnNXg==", + "dev": true, + "license": "BlueOak-1.0.0", + "dependencies": { + "lru-cache": "^11.0.0", + "minipass": "^7.1.2" + }, + "engines": { + "node": "18 || 20 || >=22" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, + "node_modules/require-from-string": { + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/require-from-string/-/require-from-string-2.0.2.tgz", + "integrity": "sha512-Xf0nWe6RseziFMu+Ap9biiUbmplq6S9/p+7w7YXP/JBHhrUDDUhwa+vANyubuqfZWTveU//DYVGsDG7RKL/vEw==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=0.10.0" + } + }, + "node_modules/shebang-command": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/shebang-command/-/shebang-command-2.0.0.tgz", + "integrity": "sha512-kHxr2zZpYtdmrN1qDjrrX/Z1rR1kG8Dx+gkpK1G4eXmvXswmcE1hTWBWYUzlraYw1/yZp6YuDY77YtvbN0dmDA==", + "dev": true, + "license": "MIT", + "dependencies": { + "shebang-regex": "^3.0.0" + }, + "engines": { + "node": ">=8" + } + }, + "node_modules/shebang-regex": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/shebang-regex/-/shebang-regex-3.0.0.tgz", + "integrity": "sha512-7++dFhtcx3353uBaq8DDR4NuxBetBzC7ZQOhmTQInHEd6bSrXdiEyzCvG07Z44UYdLShWUyXt5M/yhz8ekcb1A==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=8" + } + }, + "node_modules/signal-exit": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-4.1.0.tgz", + "integrity": "sha512-bzyZ1e88w9O1iNJbKnOlvYTrWPDl46O1bG0D3XInv+9tkPrxrN8jUUTiFlDkkmKWgn1M6CfIA13SuGqOa9Korw==", + "dev": true, + "license": "ISC", + "engines": { + "node": ">=14" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, + "node_modules/which": { + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/which/-/which-2.0.2.tgz", + "integrity": "sha512-BLI3Tl1TW3Pvl70l3yq3Y64i+awpwXqsGBYWkkqMtnbXgrMD+yj7rhW0kuEDxzJaYXGjEW5ogapKNMEKNMjibA==", + "dev": true, + "license": "ISC", + "dependencies": { + "isexe": "^2.0.0" + }, + "bin": { + "node-which": "bin/node-which" + }, + "engines": { + "node": ">= 8" + } + } + } +} diff --git a/testing/package.json b/testing/package.json new file mode 100644 index 0000000..4478e3e --- /dev/null +++ b/testing/package.json @@ -0,0 +1,14 @@ +{ + "name": "agent-skills-testing", + "private": true, + "type": "module", + "description": "Shared test tooling for agent-skills evals (schema validation of evals.json).", + "scripts": { + "validate:evals": "node validate-evals.mjs", + "test": "node --test validate-evals.test.mjs lint-item-echo.test.mjs" + }, + "devDependencies": { + "ajv": "^8.17.1", + "glob": "^11.0.0" + } +} diff --git a/testing/qa-resolve-skills.test.sh b/testing/qa-resolve-skills.test.sh new file mode 100644 index 0000000..d00da81 --- /dev/null +++ b/testing/qa-resolve-skills.test.sh @@ -0,0 +1,101 @@ +#!/usr/bin/env bash +# Regression test for .github/scripts/qa-resolve-skills.sh's PR-mode resolution, with +# particular focus on the deleted/renamed-skill path. +# +# The resolver previously derived the changed-skill list from the HEAD tree only, so a PR +# that deleted or renamed skills//SKILL.md removed that skill from the head-derived +# ALL_SKILLS, its changed path was dropped, `skills=[]` was emitted, and the seeder posted +# "no skill changes" SUCCESS — letting the deletion bypass the required qa-eval gate. +# These cases pin the fix: removals under skills/ resolve to a separate deleted_skills +# output (runnable head skills stay in skills), and never to a silent "no changes". +# +# Run: bash testing/qa-resolve-skills.test.sh (needs jq; gh is stubbed to a fixture) +set -u + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +RESOLVER="$SCRIPT_DIR/.github/scripts/qa-resolve-skills.sh" +WORK=$(mktemp -d) +trap 'rm -rf "$WORK"' EXIT +cd "$WORK" + +# Simulated HEAD tree: 'keep' and 'other' exist; 'doomed' and 'old' are the base-only +# skills this PR removed (so they must NOT exist on disk). +mkdir -p skills/keep skills/other skills/mongodb-connection testing/keep/evals +for s in keep other mongodb-connection; do echo "# ${s}" > "skills/$s/SKILL.md"; done +touch testing/keep/evals/evals.json + +# Stub gh api: keyed on the PR number, returns Git's per-file filenamestatus TSV. +cat > gh <<'STUB' +#!/usr/bin/env bash +case "$*" in + */pulls/1/*) printf 'skills/doomed/SKILL.md\tremoved\n';; + */pulls/2/*) printf 'README.md\tmodified\n';; + */pulls/3/*) printf 'skills/new/SKILL.md\trenamed\tskills/old/SKILL.md\nskills/keep/SKILL.md\tmodified\t\n';; + */pulls/4/*) printf 'skills/mongodb-connection/SKILL.md\tmodified\n';; + */pulls/5/*) printf 'skills/mongodb-connection/SKILL.md\tmodified\nskills/keep/SKILL.md\tmodified\n';; + */pulls/6/*) printf 'skills/other/new.md\trenamed\tskills/keep/old.md\n';; + *) exit 1;; +esac +STUB +chmod +x gh +export GITHUB_REPOSITORY=agent-skills-test/agent-skills +export PATH="$WORK:$PATH" + +failures=0 +check() { + local pr="$1" exp_skills="$2" exp_deleted="$3" + local out="$WORK/out.$pr" + export GITHUB_OUTPUT="$out" + bash "$RESOLVER" --pr "$pr" >/dev/null 2>&1 + local s d u + s=$(sed -n 's/^skills=//p' "$out") + d=$(sed -n 's/^deleted_skills=//p' "$out") + u=$(sed -n 's/^unsupported_skills=//p' "$out") + if [ "$s" != "$exp_skills" ] || [ "$d" != "$exp_deleted" ] || [ "$u" != "[]" ]; then + echo "FAIL pr=$pr: skills=$s (want $exp_skills), deleted=$d (want $exp_deleted), unsupported=$u (want [])" + failures=$((failures + 1)) + else + echo "ok pr=$pr: skills=$s deleted=$d unsupported=$u" + fi +} + +echo "1) a skill deleted outright must resolve to deleted_skills, never a silent no-change" +check 1 '[]' '["doomed"]' +echo "2) non-skill changes only -> genuinely no skill changes" +check 2 '[]' '[]' +echo "3) a rename (old removed, new added) + a real edit -> eval keeps, flag old" +check 3 '["keep"]' '["old"]' +echo "4) capability classification stays out of the public resolver" +check 4 '["mongodb-connection"]' '[]' +echo "5) every changed skill is returned for private classification" +check 5 '["keep","mongodb-connection"]' '[]' +echo "6) a renamed file evaluates both existing source and destination skills" +check 6 '["keep","other"]' '[]' + +export GITHUB_OUTPUT="$WORK/out.explicit" +if bash "$RESOLVER" --skill mongodb-connection >/dev/null 2>&1; then + s=$(sed -n 's/^skills=//p' "$WORK/out.explicit") + u=$(sed -n 's/^unsupported_skills=//p' "$WORK/out.explicit") + if [ "$s" = '["mongodb-connection"]' ] && [ "$u" = '[]' ]; then + echo "ok --skill existing -> public resolver returns the skill" + else + echo "FAIL --skill existing: skills=$s unsupported=$u" + failures=$((failures + 1)) + fi +else + echo "FAIL --skill mongodb-connection exited nonzero" + failures=$((failures + 1)) +fi +# And a genuinely missing skill name still fails loud. +if bash "$RESOLVER" --skill does-not-exist >/dev/null 2>&1; then + echo "FAIL --skill does-not-exist should have exited nonzero" + failures=$((failures + 1)) +else + echo "ok --skill missing fails loud" +fi + +if [ "$failures" -gt 0 ]; then + echo "$failures resolver regression case(s) failed" + exit 1 +fi +echo "all cases passed" diff --git a/testing/validate-evals.mjs b/testing/validate-evals.mjs new file mode 100644 index 0000000..dde6864 --- /dev/null +++ b/testing/validate-evals.mjs @@ -0,0 +1,255 @@ +#!/usr/bin/env node +/** + * Validate every testing//evals/evals.json against testing/evals.schema.json. + * + * Reports EVERY violation across all files, then exits 1 — deliberately not first-failure, + * so an author fixes the whole set in one pass instead of rediscovering the next problem on + * each CI run. Each violation prints the file path plus the JSON path within it (ajv's + * instancePath), so a malformed eval case fails where it was authored. + * + * Run: `npm ci --prefix testing && node testing/validate-evals.mjs` — `ci`, not `install`, + * to match .github/workflows/validate-eval-cases.yml exactly; a local run that resolves + * different dependency versions than CI is a local run that can disagree with it. + * CWD-independent: paths resolve relative to this file, so it works from the repo root or + * from testing/. + */ +import { readFileSync, realpathSync, statSync } from "node:fs"; +import { dirname, isAbsolute, join, relative, resolve } from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { globSync } from "glob"; +// draft 2020-12: the schema declares $schema 2020-12, so use Ajv2020 (default Ajv is draft-07). +import Ajv2020 from "ajv/dist/2020.js"; + +const here = dirname(fileURLToPath(import.meta.url)); + +/** + * Parse JSON with the filename in the message. The whole point of this script is that a + * broken case file produces a one-line error pointing at the problem in this PR; letting + * JSON.parse throw bare gives a stack trace with no filename -- exactly the failure mode + * it exists to prevent. + */ +function readJson(file) { + const text = readFileSync(file, "utf8"); + try { + return JSON.parse(text); + } catch (err) { + console.error(`\u2717 ${file}\n not valid JSON: ${err.message}`); + process.exit(1); + } +} + +// The seed/fixture naming contract comes from the schema (x-fixtureContract) rather than +// being hardcoded here, because agent-skills-evals/solvers/seed_db.py resolves fixtures +// against the same convention at run time. Two independent hardcoded copies is how a case +// starts passing this check and then failing to seed inside the harness. +function contractFrom(schema) { + const contract = schema["x-fixtureContract"] ?? {}; + return { + FIXTURE_DIR: contract.fixtureDir ?? "fixtures", + FIXTURE_EXT: contract.fixtureExtension ?? ".js", + RESERVED_SEEDS: new Set(contract.reservedSeeds ?? ["clean_slate"]), + }; +} + +/** + * Case ids must be unique within a skill's file: the harness keys qa_runs metadata and the + * mutation/sensitivity analysis on the id, so a duplicate merges two unrelated cases into + * one identity (see agent-skills-evals's test_case_id_identity.py for the corruption shape). + * ajv cannot express this — uniqueItems compares WHOLE items, and two cases sharing an id + * differ in prompt — so the rule lives here with the other checks the schema cannot do. + * + * Exported so the rule is pinned by validate-evals.test.mjs, same as filesEntryEscapes. + */ +export function duplicateIds(doc) { + const problems = []; + const seen = new Set(); + for (const ev of Array.isArray(doc.evals) ? doc.evals : []) { + if (ev.id === undefined) continue; // missing id is the schema's finding, not this one's + if (seen.has(ev.id)) { + problems.push(`case id ${ev.id} is used twice in this file`); + } + seen.add(ev.id); + } + return problems; +} + +/** + * Does a `files` entry escape the case's own evals dir? Pure (no fs), so the eval-integrity + * rule the schema regex alone cannot express is unit-testable. + * + * The schema pattern rejects a leading '/' and any '..' segment, but a pattern cannot reason + * about what a path RESOLVES to. This is the authoritative containment check. It is what + * blocks cross-skill answer-key handover — a case pointing at ../mongodb-other-skill/evals/ + * or ../../skills//SKILL.md would hand the agent its own answer key and be invisible + * to lint-item-echo.mjs (which excludes `files` from overlap analysis because they are + * provided input data). + * + * resolve() rather than join() because join() disagrees with the harness on absolute paths: + * join('/a', '/etc/x') nests to '/a/etc/x', while Python's Path('/a') / '/etc/x' honours the + * absolute and yields '/etc/x'. resolve() matches the harness, so the two agree on what is + * being checked. + */ +export function filesEntryEscapes(evalsDir, ref) { + const resolved = resolve(evalsDir, ref); + const rel = relative(evalsDir, resolved); + // rel === "" (e.g. files: ["."]) is NOT an escape: the ref resolves to the evals dir + // itself, which is contained. It is a directory, and checkAssetsExist's isFile() guard + // rejects it with the correct classification. + return isAbsolute(ref) || rel.startsWith("..") || isAbsolute(rel); +} + +// A case's `files` entries and `seed` name point at real paths, but nothing in the schema +// itself can check a path exists. Missing here means agent-skills-evals's case_source.py +// (build_prompt) or seed_db.py would only discover it deep inside a harness run, as a +// FileNotFoundError instead of a one-line message pointing at the bad path in this PR. +// +// Exported (and takes the fixture contract as a param) so the containment rule is pinned by +// validate-evals.test.mjs — without that, the cross-skill-escape guard is code that only runs +// in CI and has no test that fails when it regresses. +export function checkAssetsExist(evalsDir, doc, contract) { + const { FIXTURE_DIR, FIXTURE_EXT, RESERVED_SEEDS } = contract; + const problems = []; + for (const ev of Array.isArray(doc.evals) ? doc.evals : []) { + for (const ref of Array.isArray(ev.files) ? ev.files : []) { + // Containment BEFORE existence. A path that escapes is rejected whether or not the + // target happens to exist — the answer-key file under skills// DOES exist. + const resolved = resolve(evalsDir, ref); + if (filesEntryEscapes(evalsDir, ref)) { + problems.push( + `case ${ev.id}: files entry escapes the evals dir: ${ref} -> ${resolved}. ` + + `Assets must live under ${evalsDir}.`, + ); + } else { + let assetStat; + try { + assetStat = statSync(resolved); + } catch (err) { + const detail = + err.code === "ENOENT" + ? `files entry not found: ${resolved}` + : `could not inspect files entry ${resolved}: ${err.message}`; + problems.push(`case ${ev.id}: ${detail}`); + continue; + } + if (!assetStat.isFile()) { + // `files` are asset files the harness inlines into the prompt. A directory fails + // deep inside a run as EISDIR instead of here, one line, in this PR. statSync + // follows symlinks, so a symlink to a directory is caught here as well. + problems.push(`case ${ev.id}: files entry is a directory, not a file: ${resolved}`); + continue; + } + // And again after following symlinks. A symlink inside evals/ needs no '..' and no + // leading '/', so it satisfies both the schema pattern and the resolve() check above + // while still pointing anywhere. realpathSync can throw — ELOOP from a symlink + // cycle, or a race where the path disappears between the stat above and this + // resolve. That is a per-case problem to report, not a reason to crash the whole + // validator and swallow every later finding. + let realEvalsDir; + let realResolved; + try { + realEvalsDir = realpathSync(evalsDir); + realResolved = realpathSync(resolved); + } catch (err) { + problems.push( + `case ${ev.id}: could not resolve the real path of ${resolved}: ${err.message}`, + ); + continue; + } + const realRel = relative(realEvalsDir, realResolved); + if (realRel.startsWith("..") || isAbsolute(realRel)) { + problems.push( + `case ${ev.id}: files entry resolves outside the evals dir via a symlink: ` + + `${ref} -> ${realResolved}`, + ); + } + } + } + if (ev.seed && !RESERVED_SEEDS.has(ev.seed)) { + const fixtureDir = join(evalsDir, FIXTURE_DIR); + const fixture = join(fixtureDir, `${ev.seed}${FIXTURE_EXT}`); + let fixtureStat; + try { + fixtureStat = statSync(fixture); + } catch (err) { + const detail = + err.code === "ENOENT" + ? `has no fixture at ${fixture}` + : `fixture could not be inspected at ${fixture}: ${err.message}`; + problems.push(`case ${ev.id}: seed '${ev.seed}' ${detail}`); + continue; + } + if (!fixtureStat.isFile()) { + problems.push(`case ${ev.id}: seed fixture is not a regular file: ${fixture}`); + } else { + // Same realpath containment that `files` gets: `files` are inlined into the prompt + // and seed fixtures are read and copied into the sandbox by the harness, so a + // symlink `fixtures/leak.js -> /proc/self/environ` would hand runner-local data to + // the privileged eval runner. The schema's `seed` regex only constrains the NAME — + // it cannot see what the file RESOLVES to. statSync above already follows a symlink, + // so this branch's own realpath comparison is the authoritative containment check. + let realFixtureDir; + let realFixture; + try { + realFixtureDir = realpathSync(fixtureDir); + realFixture = realpathSync(fixture); + } catch (err) { + problems.push( + `case ${ev.id}: could not resolve the real path of seed fixture ` + + `${fixture}: ${err.message}`, + ); + continue; + } + const realRel = relative(realFixtureDir, realFixture); + if (realRel.startsWith("..") || isAbsolute(realRel)) { + problems.push( + `case ${ev.id}: seed fixture resolves outside the fixtures dir via a symlink: ` + + `${fixture} -> ${realFixture}`, + ); + } + } + } + } + return problems; +} + +function main() { + const schema = readJson(join(here, "evals.schema.json")); + const ajv = new Ajv2020({ allErrors: true, strict: false }); + const validate = ajv.compile(schema); + const contract = contractFrom(schema); + + const files = globSync(join(here, "*/evals/evals.json")).sort(); + let failed = false; + + for (const file of files) { + const doc = readJson(file); + const schemaOk = validate(doc); + const assetProblems = checkAssetsExist(dirname(file), doc, contract); + const idProblems = duplicateIds(doc); + + if (schemaOk && assetProblems.length === 0 && idProblems.length === 0) { + console.log(`✓ ${file} (${doc.evals?.length ?? 0} cases)`); + } else { + failed = true; + console.error(`✗ ${file}`); + for (const e of validate.errors ?? []) { + console.error(` ${e.instancePath || "(root)"}: ${e.message}`); + } + for (const p of assetProblems) { + console.error(` ${p}`); + } + for (const p of idProblems) { + console.error(` ${p}`); + } + } + } + + if (files.length === 0) { + console.error("No testing/*/evals/evals.json found."); + process.exit(1); + } + process.exit(failed ? 1 : 0); +} + +// Run as a CLI only when invoked directly, not when imported by the test. +if (import.meta.url === pathToFileURL(process.argv[1] ?? "").href) main(); diff --git a/testing/validate-evals.test.mjs b/testing/validate-evals.test.mjs new file mode 100644 index 0000000..d7494e8 --- /dev/null +++ b/testing/validate-evals.test.mjs @@ -0,0 +1,191 @@ +// Pins the `files` containment rule that the evals.schema.json regex alone cannot enforce. +// +// The schema pattern rejects a leading '/' and any '..' segment, but a regex cannot reason +// about what a path RESOLVES to. validate-evals.mjs::filesEntryEscapes is the authoritative +// check, and it is what blocks cross-skill answer-key handover: a case pointing at another +// skill's evals/ or at ../../skills//SKILL.md would hand the agent its own answer key, +// pass the schema regex, and be invisible to lint-item-echo.mjs (which excludes `files` from +// overlap analysis because they are provided input data). Without this test that guard is +// code that only runs in CI with no failure when it regresses. +// +// Uses node's built-in test runner — no new dependency on the testing/ toolchain. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, mkdirSync, writeFileSync, symlinkSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { filesEntryEscapes, checkAssetsExist, duplicateIds } from "./validate-evals.mjs"; + +const EVALS_DIR = "/repo/testing/mongodb-query-optimizer/evals"; +const CONTRACT = { + FIXTURE_DIR: "fixtures", + FIXTURE_EXT: ".js", + RESERVED_SEEDS: new Set(["clean_slate"]), +}; + +test("filesEntryEscapes: paths inside the evals dir are contained", () => { + assert.equal(filesEntryEscapes(EVALS_DIR, "fixtures/seeded_events.js"), false); + assert.equal(filesEntryEscapes(EVALS_DIR, "asset.json"), false); + assert.equal(filesEntryEscapes(EVALS_DIR, "sub/dir/asset.json"), false); +}); + +test("filesEntryEscapes: parent traversal escapes — the cross-skill answer-key handover", () => { + // Reaching another skill's evals dir requires '..', which the schema regex forbids as a + // segment — but a regex forbidding '..' and a resolve() that catches '..' are two different + // defenses. This pins the latter, which is the one that reasons about the resolved target. + assert.equal( + filesEntryEscapes(EVALS_DIR, "../mongodb-search-and-ai/evals/seeded_movies.js"), + true, + ); + assert.equal( + filesEntryEscapes(EVALS_DIR, "../../skills/mongodb-query-optimizer/SKILL.md"), + true, + ); +}); + +test("filesEntryEscapes: absolute paths escape (resolve, not join, semantics)", () => { + // join('/a', '/etc/x') nests to '/a/etc/x'; resolve() honours the absolute and yields + // '/etc/x'. The harness uses Path('/a') / '/etc/x' (resolve semantics), so this matches. + assert.equal(filesEntryEscapes(EVALS_DIR, "/etc/passwd"), true); +}); + +test("filesEntryEscapes: '.' is contained, not an escape (misclassification regression)", () => { + // Resolving "." yields the evals dir itself (rel === ""). That is contained; the entry is + // invalid because it is a DIRECTORY, and classifying it as an escape sends the author + // chasing a traversal that does not exist. checkAssetsExist's isFile() guard rejects it. + assert.equal(filesEntryEscapes(EVALS_DIR, "."), false); + assert.equal(filesEntryEscapes(EVALS_DIR, "sub/dir/."), false); +}); + +test("checkAssetsExist: a cross-skill files ref is rejected even when the target exists", () => { + // The answer-key file under another skill DOES exist — existence is not the point, + // containment is. This is the case the schema regex + an existence-only check would miss. + const root = mkdtempSync(join(tmpdir(), "evals-")); + const skillA = join(root, "skill-a", "evals"); + const skillB = join(root, "skill-b", "evals"); + mkdirSync(skillA, { recursive: true }); + mkdirSync(skillB, { recursive: true }); + writeFileSync(join(skillB, "answer.md"), "the answer key"); + const doc = { evals: [{ id: 1, files: ["../../skill-b/evals/answer.md"] }] }; + const problems = checkAssetsExist(skillA, doc, CONTRACT); + assert.ok( + problems.some((p) => p.includes("escapes the evals dir")), + `expected an escape finding, got: ${JSON.stringify(problems)}`, + ); +}); + +test("checkAssetsExist: a contained, existing file is accepted", () => { + const root = mkdtempSync(join(tmpdir(), "evals-")); + const evalsDir = join(root, "evals"); + mkdirSync(evalsDir, { recursive: true }); + writeFileSync(join(evalsDir, "asset.json"), "{}"); + const doc = { evals: [{ id: 1, files: ["asset.json"] }] }; + assert.deepEqual(checkAssetsExist(evalsDir, doc, CONTRACT), []); +}); + +test("checkAssetsExist: a files entry pointing at a directory is rejected", () => { + // existsSync() treats directories as existing, but `files` are asset files the harness + // inlines. A directory would validate here and fail downstream as EISDIR. + const root = mkdtempSync(join(tmpdir(), "evals-")); + const evalsDir = join(root, "evals"); + mkdirSync(join(evalsDir, "assets"), { recursive: true }); + const doc = { evals: [{ id: 1, files: ["assets"] }] }; + const problems = checkAssetsExist(evalsDir, doc, CONTRACT); + assert.ok( + problems.some((p) => p.includes("directory, not a file")), + `expected a not-a-file finding, got: ${JSON.stringify(problems)}`, + ); +}); + +test("checkAssetsExist: a missing contained file is reported as not-found (not as an escape)", () => { + const root = mkdtempSync(join(tmpdir(), "evals-")); + const evalsDir = join(root, "evals"); + mkdirSync(evalsDir, { recursive: true }); + const doc = { evals: [{ id: 1, files: ["missing.json"] }] }; + const problems = checkAssetsExist(evalsDir, doc, CONTRACT); + assert.ok(problems.some((p) => p.includes("not found"))); + assert.ok(problems.every((p) => !p.includes("escapes"))); +}); + +test("duplicateIds: a reused case id is reported", () => { + // The harness keys qa_runs metadata and mutation analysis on the id; a duplicate merges + // two unrelated cases into one identity. ajv cannot express this (uniqueItems compares + // whole items), so the check lives in validate-evals.mjs and is pinned here. + const doc = { evals: [{ id: 7, prompt: "a" }, { id: 8, prompt: "b" }, { id: 7, prompt: "c" }] }; + assert.deepEqual(duplicateIds(doc), ["case id 7 is used twice in this file"]); +}); + +test("duplicateIds: unique ids and id-less cases are clean", () => { + // Missing ids are the schema's finding (id is required), not this check's. + assert.deepEqual(duplicateIds({ evals: [{ id: 1 }, { id: 2 }] }), []); + assert.deepEqual(duplicateIds({ evals: [{ prompt: "no id yet" }] }), []); +}); + +test("duplicateIds: a non-array evals value does not crash the validator", () => { + // The schema expects doc.evals to be an array; a malformed case that puts e.g. an object + // there used to throw a TypeError and stop the validator before it printed the schema + // errors — defeating the "report EVERY violation" behavior the header promises. + assert.deepEqual(duplicateIds({ evals: "not-an-array" }), []); + assert.deepEqual(duplicateIds({ evals: { id: 1 } }), []); +}); + +test("checkAssetsExist: malformed evals or files shapes do not crash the validator", () => { + // Same crash class as duplicateIds: files: 123 (or evals: {...}) used to throw a TypeError + // mid-scan instead of being reported as schema violations. The Array.isArray guards keep + // the validator from failing before it can print everything. + const root = mkdtempSync(join(tmpdir(), "evals-")); + const evalsDir = join(root, "evals"); + mkdirSync(evalsDir, { recursive: true }); + assert.deepEqual(checkAssetsExist(evalsDir, { evals: "not-an-array" }, CONTRACT), []); + assert.deepEqual( + checkAssetsExist(evalsDir, { evals: [{ id: 1, files: "not-an-array" }] }, CONTRACT), + [], + ); +}); + +test("checkAssetsExist: a seed fixture symlink that escapes the fixtures dir is rejected", () => { + // `files` get realpath containment but seed fixtures previously got only an existence + // check — a git symlink `fixtures/leak.js -> /proc/self/environ` validated clean and the + // harness then copied runner-local data into the sandbox. Pin the containment rule that + // closes it: the resolved fixture must stay inside the resolved fixtures dir. + const root = mkdtempSync(join(tmpdir(), "evals-")); + const evalsDir = join(root, "evals"); + mkdirSync(join(evalsDir, "fixtures"), { recursive: true }); + writeFileSync(join(root, "victim.js"), "const secret = 'GROVE_API_KEY=leaked'\n"); + // fixtures/ -> evals/ -> root/ : two levels up reaches the victim, outside the fixtures dir + symlinkSync("../../victim.js", join(evalsDir, "fixtures", "leak.js")); + const doc = { evals: [{ id: 1, seed: "leak" }] }; + const problems = checkAssetsExist(evalsDir, doc, CONTRACT); + assert.ok( + problems.some((p) => p.includes("outside the fixtures dir via a symlink")), + `expected a symlink-escape finding, got: ${JSON.stringify(problems)}`, + ); +}); + +test("checkAssetsExist: a contained non-symlink seed fixture is accepted", () => { + const root = mkdtempSync(join(tmpdir(), "evals-")); + const evalsDir = join(root, "evals"); + mkdirSync(join(evalsDir, "fixtures"), { recursive: true }); + writeFileSync(join(evalsDir, "fixtures", "seeded_movies.js"), "const s = 1;\n"); + const doc = { evals: [{ id: 1, seed: "seeded_movies" }] }; + assert.deepEqual(checkAssetsExist(evalsDir, doc, CONTRACT), []); +}); + +test("checkAssetsExist: a symlink loop is reported, never thrown", () => { + // The ELOOP case Copilot raised about realpathSync: a pathological symlink cycle must + // surface as a per-case problem, not crash the validator and swallow the remaining + // findings. Which branch observes the pathology is platform-dependent — node's + // existsSync reports a cycle here as missing, so it lands on the not-found finding; on + // setups where the earlier gates let it through, the realpath guard reports it. Either + // way the contract is: a problem list comes back, nothing throws. + const root = mkdtempSync(join(tmpdir(), "evals-")); + const evalsDir = join(root, "evals"); + mkdirSync(evalsDir, { recursive: true }); + symlinkSync(join(evalsDir, "beta"), join(evalsDir, "alpha")); + symlinkSync(join(evalsDir, "alpha"), join(evalsDir, "beta")); + const doc = { evals: [{ id: 1, files: ["alpha"] }] }; + const problems = checkAssetsExist(evalsDir, doc, CONTRACT); + assert.ok(Array.isArray(problems)); + assert.ok(problems.length > 0, "a cycle should be reported, not treated as clean"); +});