skills/mongodb__agent-skills
1
0
Fork 0
mirror of https://github.com/mongodb/agent-skills.git synced 2026-09-18 21:15:11 +08:00
Code Issues Packages Projects Releases Wiki Activity

proposal: functional-grading fields + schema for testing/*/evals.json (draft) (#55)

Browse Source 
* proposal: optional functional-grading fields + schema for testing/*/evals.json

Adds optional seed/functional_check/trajectory_checks to the eval-case format plus a JSON
Schema and a hermetic fixtures/ convention, so the automated Inspect QA harness can grade a
skill against a real (local) MongoDB — complementary to the existing setup-slow-queries.ts /
skill-creator live-Atlas workflow, not a replacement. Fully backward compatible: all fields
optional; all current cases validate unchanged.

- testing/evals.schema.json: JSON Schema (draft 2020-12) for evals.json (current shape + the
  new optional functional fields).
- testing/validate-evals.mjs: ajv validator to wire into CI (left for the team to hook into
  validate-skills.yml).
- testing/mongodb-query-optimizer: a sample functional case (id 20, ESR compound index, graded
  on the query plan) + its hermetic fixture (seeds/seeded_events.js).

See PR description for the parallel-tiers-vs-converge question.

* ci: wire eval-case schema validation into CI (validate-eval-cases.yml)

Don't leave the wiring to reviewers. Adds a testing/**-scoped GitHub Actions workflow that
installs the validator deps and runs testing/validate-evals.mjs, so a malformed evals.json
(incl. the optional functional_check/trajectory_checks) fails in CI where it's authored.
- testing/package.json: ajv + glob devDeps + a validate:evals script.
- validate-evals.mjs: use Ajv2020 (schema is draft 2020-12; default Ajv is draft-07) and
  resolve paths relative to the file (CWD-independent).
Verified locally: npm install --prefix testing + node testing/validate-evals.mjs → all 7
evals.json (94 cases) valid, exit 0.

* ci: commit testing lockfile + use npm ci for deterministic validator deps

* feat(testing): extend eval schema for called_successfully trajectory checks

Adds "called_successfully" to trajectory_check.type's enum and a
matcher.success property, mirroring the same schema extension in the
Inspect harness repo (10gen/agent-skills-evals). Without this, an eval
case authoring a called_successfully or success: check would fail schema
validation -- the harness gained the capability (joining a tool call to
its result so "attempted" and "succeeded" are distinguishable) before the
schema was updated to let a case actually use it.

Backward compatible: all existing cases validate unchanged, since both
additions are optional.

* test: relocate DB seed fixtures from agent-skills-evals into evals/fixtures/

Fixtures backing functional_check cases (seeded_movies.js, seeded_nlq_lift.js,
seeded_sample.js for mongodb-natural-language-querying; seeded_movies_search.js for
mongodb-search-and-ai) were living in agent-skills-evals/inspect/seeds/, disconnected from
the evals.json cases that reference them by name. This is where an author is already
working when authoring a functional_check case, so the fixture now lives in the same PR/
review as the case that depends on it. mongodb-query-optimizer's seeded_events.js already
had an (identical, unused) copy here from an earlier pass; no change needed there.

agent-skills-evals now resolves fixtures from this location by default (SEED_BASE_DIR),
mirroring how SKILL_BASE_DIR already resolves the skill content itself from this repo.

* test: relocate mutation battery config from agent-skills-evals

Mirrors 8351f8e's seed-fixture move: invert_rule/key_reference config for the mutation
battery was living in agent-skills-evals/cases/qa/<skill>/mutation.yaml, disconnected from
SKILL.md and evals.json. Now lives alongside evals.json, so an author authoring/updating a
skill's mutation battery works in this repo, in one PR, without a separate checkout.

* Draft mutation-battery CI workflow (workflow_dispatch only)

Not yet wired to push/schedule triggers -- pending the Grove-secret
custody review and extending the agent-skills-evals GitHub App's
installation to this repo. See in-file header for details.

* Add asset-existence check and answer-echo lint to eval-case validation

validate-evals.mjs now confirms every case's files[]/seed reference a real
path, instead of failing deep inside the QA harness (case_source.py /
seed_db.py) with no hint which PR broke it.

lint-item-echo.mjs flags eval items whose authored text (prompt/
expected_output/expectations) overlaps heavily with their own skill's
SKILL.md/references -- i.e. items that test whether the agent can quote
the guidance back rather than apply it. Threshold is calibrated from the
corpus's own cross-skill null distribution rather than a guessed constant.
Advisory (reports, doesn't block) until checked against a few real PRs.

* Draft per-PR QA screen-check workflow (workflow_dispatch only)

Tier 2/3 of DESIGN.md's tiering -- runs skill-off then skill-on (k=1) for
a changed skill and posts a sanitized PR comment via
agent-skills-evals/inspect/screen_report.py. Same two blockers as
qa-mutation-battery.yml (Grove-secret custody review; GitHub App
install-scope extension), so pull_request stays commented out in favor
of workflow_dispatch until both land. Tier 1 (schema/lint/drift/echo) is
already live elsewhere and this workflow doesn't duplicate it.

* Correct GitHub App requirement in draft workflow comments

Cross-repo checkout via actions/create-github-app-token only needs the
App installed on the TARGET repo (agent-skills-evals, already true) --
not on the repo the workflow runs in. What these drafts actually still
need is AGENT_SKILLS_EVALS_APP_ID/_PRIVATE_KEY as Actions secrets on this
repo, unrelated to the App's installation scope.

(The App genuinely does need agent-skills in its installation for a
different, unrelated consumer -- agent-skills-evals's T1 GitHubReportWriter,
which commits eval-report.json/md there directly.)

* fix(ci): make the non-T1 eval jobs runnable and the echo lint honest

qa-mutation-battery ran the whole battery at --epochs 1. This repo's own noise
measurement says that cannot answer anything: at k=2 a NULL control (zero damage)
produced a per-item S_i of 0.114, above the 0.10 gate threshold, and only at k=5
did the floor fall to 0.029. mutation_report correctly suppresses the MDD in that
regime, so the job spent a full battery of model calls per skill to publish
"unmeasurable". Default is now 5, exposed as a workflow input with the reasoning
in its description.

Also in that workflow: every matrix job pushed its own baseline commit to
agent-skills-evals main, so with fail-fast:false the first would win and the rest
fail on non-fast-forward -- a 7-skill census quietly losing most of its results.
Reports are now artifacts, committed once by a `publish` job with rebase-retry.
Plus timeout-minutes (~11 mutants at k=5 vs. a 6h default), a concurrency group,
MAX_SAMPLES 2 (it is a CONCURRENCY cap and each sample spins its own agent+mongo
pair -- 4 is 8 containers on a 2-vCPU runner), `grep || true` so a no-match under
pipefail yields [] instead of failing the job, and candidate names intersected
against ALL_SKILLS so testing/skills-boundaries cannot become a matrix entry.

qa-pr-screen: documents fork PRs as a third blocker before the pull_request
trigger goes live. This repo is public and takes community contributions, and a
fork run gets no secrets -- so every external PR would get a red X on an
explicitly advisory check. Handled with a fork-notice job and same-repo scoping;
pull_request_target is the alternative that does get secrets, and running an
untrusted PR head with credentials needs a maintainer gate, which is a decision
to make rather than a default to fall into. Also replaces `always()` (which would
evaluate fromJSON('') after a failed detect), adds a concurrency group so two
pushes cannot double-post the marker comment, and passes --model explicitly since
a drift from screen_report's default surfaces as "no qa_runs doc found".

lint-item-echo claimed its threshold was "calibrated from the corpus's own
cross-skill null rather than a guessed constant". Measured: the null is p99=0.000
(two skills essentially never share an 8-gram), so max(p99, 0.15) always resolved
to the constant it said it was avoiding. Comment now says what is true, and the
floor is named as the operative threshold.

More substantively, the lint pooled prompt and expected_output into one score.
Those mean opposite things -- overlap in the prompt is leakage, overlap in the
expected answer is often just a correctly-stated answer -- so pooling mostly
measured the second and reported it as the first. Now split per field, which
surfaces one finding the pooled version missed. Adds the co-movement check ported
from the private echo.py: an edit that RAISES an item's containment is teaching to
the test, detectable from the diff alone with no held-out set, which makes it the
highest-value check available to a per-PR job. Wired to --base in CI.

Smaller: both validators now report a bad JSON file with its filename instead of
an unhandled stack trace; the echo lint fails loudly when skill_name matches no
skills/ directory, rather than reporting a green "no echoing items" over an empty
corpus; the --strict TODO is dated and owned. New echo-thresholds.json and the
schema's x-fixtureContract single-source the constants and the seed/fixture
convention that agent-skills-evals also resolves against at run time.

* Add qa-eval-status and qa-eval-check workflows

Promotes the reference workflows from agent-skills-evals/docs/ (the T2
QA eval flow's PR-gate half) into real, live workflows here -- the last
item from that flow's "not yet implemented" checklist that was mine to
do. Renamed to Title Case and bumped actions/checkout to v6 to match
this repo's existing workflow conventions; otherwise unchanged from the
reference.

These post a `qa-eval` status context but don't block merges on their
own -- main has no branch-protection required-status-checks configured
today, so this is visible-but-non-enforcing until someone separately
adds "qa-eval" as a required check.

* Pin GitHub Actions to commit SHAs (Semgrep github-actions-mutable-action-tag)

14 findings on PR #55, all the same rule: a mutable major-version tag
(@v6, @v4, @v2) can be silently repointed by the action owner (or an
attacker who compromises their account) without this repo's history
changing at all -- the cited precedent is the trivy-action and
kics-github-action compromises. Pinned every actions/checkout,
create-github-app-token, setup-uv, setup-node, upload-artifact, and
download-artifact reference to its resolved commit SHA, with a
# vX.Y.Z comment for readability. Verified each SHA against the
GitHub API before using it (resolving through the annotated-tag
indirection where a tag object's own SHA isn't the commit SHA, e.g.
astral-sh/setup-uv). No behavior change -- same versions, just pinned.

* fix(ci): derive the skill list from disk, and don't claim co-movement on a new skill

Both found by walking mongodb/agent-skills#50 (a fork PR adding mongodb-laravel)
through this infra.

The ALL_SKILLS allowlist I added was a literal list of today's seven mongodb-*
skills, used to filter path-derived candidates. A skill added since -- exactly
what a community contribution is -- was not in it, so detect silently dropped it:
the PR-triggered screen would have screened nothing, and the battery's census
would have skipped it. That is worse than the deep harness failure the allowlist
replaced, because a silent skip and "no work to do" look identical.

Now derived from skills/*/ (any directory with a SKILL.md). Because pull_request
checks out the merge ref, a skill ADDED by the PR is already on disk during
detect, so it gets screened. Verified against the real tree (derives the 7,
matching skills/*/) and against a simulated merge ref containing mongodb-laravel:
kept, with skills-boundaries still dropped.

Two related fixes: qa-pr-screen now emits the ::notice for dropped candidates (it
was the only one staying silent about them), and a typo'd workflow_dispatch input
now fails with the available list rather than running an empty matrix that reports
success.

Co-movement asks whether THIS PR's guidance edit moved an item's overlap. For a
PR that adds a skill there is no before-state, so every item trivially measures
0 -> n and would be reported as a rise that never happened. readCorpusAtRef now
returns anyPresent alongside the text -- "" is ambiguous, and a file that existed
and was empty is a real before-state while a skill that did not exist is not --
and co-movement skips those skills with a stated reason. An item that copies a NEW
skill's text is still echoing; that is the static check's finding to report, in
its own words.

Verified both paths in a reconstructed merge state of #50: the new skill is
skipped with the reason printed, and appending an item's prompt into an EXISTING
skill's SKILL.md still flags co-movement (0.00 -> 1.00). #50 itself passes schema
validation and produces no echo findings.

* docs(testing): explain why only two skills have a mutation.yaml

2 of 7 reads as arbitrary, and it isn't. Documented in the canonical place
(mongodb-query-optimizer/evals/mutation.yaml's header, which schema-design's file
already points at) so it survives past the PR that introduced it, and summarised in
PR #55's description.

Four points, in the order a reader needs them:

  - Absence of the file does NOT mean the skill is unmutated. Every skill gets the
    nine SKILL.md operators with no config at all, and one with references/ gets two
    more. Verified: mongodb-natural-language-querying, which has neither references/
    nor a mutation.yaml, materialises 9 of 9 mutants including real damage
    (truncate_body 0.75 takes SKILL.md from 9,626 to 2,662 bytes).

  - What the file adds is invert_rule, the operator closest to a real guidance
    regression -- document still well-formed, advice now wrong.

  - That is only worth authoring where the inverted advice is plausible-but-wrong
    rather than incoherent, AND the skill's own cases turn on the rule. Fail the
    second and S_i = 0, indistinguishable from "the skill isn't load-bearing here".
    mutate_skill raises on a find-string that misses; nothing catches an inversion
    that turns the doc into nonsense.

  - These two satisfy both and are the two the battery has been run against:
    query-optimizer's find-strings were caught 4x by the no-op raise while being
    authored, and schema-design is the skill with measured lift (0.767 vs 0.100 at
    k=5), so at the time it was the only one where mutation could show anything.

Also records what blocks the other five -- most of their rule-shaped text is
PROCEDURAL ("always consult the reference file", "always warn about billing"), and
inverting process does not change advice the graded output reveals -- and names the
follow-up order: connection (its cases assert singleton-client and maxPoolSize
directly), then search-and-ai, then atlas-stream-processing. mcp-setup and nlq may
never warrant one.

Comment-only; all five existing inversions re-verified to still hit their live files
(3 + 2 invert_rule mutants materialise).

* docs(ci): record that the Actions qa-eval workflows are superseded by Evergreen

The fork-PR problem is not an independent blocker -- it is a consequence of where the
eval executes. If a mongodb/agent-skills Evergreen project runs the QA eval, Evergreen
posts the status under its own GitHub App (which works on fork PRs, unlike a fork's
read-only GITHUB_TOKEN) and gates outside-org patches natively. At that point these
Actions workflows are superseded.

Recorded in their headers because the failure mode is leaving BOTH in place: someone
adds the Actions `qa-eval` context to the ruleset's required checks, it never posts on a
fork PR, and with bypass_actors empty every external contribution wedges permanently --
while a green Evergreen check sits next to it. Whatever posts the status has to be the
single thing named in the ruleset, so the instruction is delete, not coexist.

qa-pr-screen gets the same note on its blocker 3, which reads as a live concern today
and will read as a stale one the moment Evergreen lands.

Comment-only.

* docs: address Copilot review comments on #55

All three were legitimate. Two are stale-comment fixes; one closes a real CI gap.

1. testing/validate-evals.mjs header claimed three things the code does not do: exit on
   the FIRST violation (it reports every file, then exits 1), print the skill name (it
   prints the file path plus ajv's instancePath), and install with `npm install` (CI uses
   `npm ci`). Corrected, and the report-everything behaviour is now stated as deliberate,
   since it is the better behaviour -- an author fixes the whole set in one pass instead of
   rediscovering the next problem on each CI run. Verified by breaking two case files: two
   failures reported, exit 1.

   The `npm ci` correction matters beyond tidiness: a documented local command that
   resolves different dependency versions than CI is a local command that can disagree
   with CI.

2. validate-eval-cases.yml's paths filter omitted testing/package-lock.json. `npm ci`
   installs from the lockfile, so a lockfile-only change -- a dependency bump, a regen --
   alters what the workflow actually runs while leaving the workflow unrun. An ajv upgrade
   that broke validation would have sailed past the check meant to catch it. Added.

3. seeded_events.js's header described an earlier design of its case: "functional case 37",
   equality lookups on `userId`, and an index-EXISTENCE scorer. The actual consumer is case
   20, whose functional_check is `type: explain` -- filter {type: 'purchase'} with sort
   {ts: -1}, requiring an IXSCAN, forbidding an in-memory sort, and capping docsExamined at
   1.1x returned. So the graded behaviour is whether the agent builds a compound index
   serving BOTH the filter and the sort, not whether any index exists. Rewritten, including
   the line-18 comment Copilot flagged as the same problem.

Also confirmed semgrep's two findings on this PR are already resolved by 31b6386 (action
pinning), and that every `uses:` across all five workflows -- including the
upload-artifact/download-artifact steps added later -- is pinned to a 40-char SHA, so the
same class of finding has not been reintroduced.

* fix(testing): keep eval-case `files` inside the evals dir

Copilot's second-pass comment, and it found a real interaction bug between two things
this PR adds. `files` entries were documented as "relative paths under the evals dir"
but nothing enforced it, and the consequence is not a filesystem concern -- it is an
eval-integrity hole:

  - the harness INLINES these files into the prompt (case_source.build_prompt), and
  - lint-item-echo.mjs deliberately EXCLUDES `files` from its overlap analysis, on the
    stated grounds that they are provided input data rather than authored text.

So a case with `files: ["../../../skills/<name>/SKILL.md"]` hands the agent its own
answer key, passes validate-evals (the file genuinely exists), and is invisible to the
echo lint. That is precisely the teaching-to-the-test failure the echo lint exists to
catch, routed around it.

Fixed in three layers, because each catches what the previous cannot:

  1. Schema pattern on `files` items rejecting a leading '/' and any '..' segment -- the
     declarative contract, so the rule lives with the field it constrains. Note the
     contrast with `seed`, whose ^[a-z0-9_]+$ already made traversal unexpressible; that
     is why this only ever affected `files`.
  2. resolve()+relative() containment in the validator. resolve() rather than join() also
     fixes a real disagreement with the harness: join('/a','/etc/x') nests to '/a/etc/x',
     while Python's Path('/a') / '/etc/x' honours the absolute and yields '/etc/x', so the
     two implementations were checking different paths.
  3. realpathSync containment after existence. A SYMLINK inside evals/ needs no '..' and
     no leading '/', so it satisfies both checks above -- verified: a
     `leak.md -> ../../../skills/<name>/SKILL.md` symlink passed cleanly until this branch
     existed. Copilot's comment named the traversal case; the symlink one was underneath it.

Verified each rejection individually: answer-key traversal, absolute path, mid-path '..'
(`assets/../../../skills/...`), and the symlink all now fail with a message naming the
escape; a legitimate relative asset still falls through to the ordinary not-found check;
the real 102-case corpus still validates clean.

* test: pin files-containment, fail-loud secret gate, echo-lint PR annotations

- validate-evals.mjs refactored to guard its CLI entry and export
  filesEntryEscapes/checkAssetsExist; new validate-evals.test.mjs (node --test, no
  new dep) pins the cross-skill answer-key-handover guard that the schema regex
  alone does not enforce. Wired into validate-eval-cases.yml + package.json.
- qa-pr-screen/qa-mutation-battery: add a Require model-API credentials step that
  fails loud (::error::) if GROVE_API_KEY/GROVE_BASE_URL are absent, so uncommenting
  the triggers without the pending secrets surfaces instead of no-oping.
- lint-item-echo.mjs emits ::warning file= annotations in CI so advisory findings
  surface in PR review (still exit 0); local output unchanged.

Validator exits 0; 6 containment tests pass; YAML valid.

Co-Authored-By: Claude <noreply@anthropic.com>

* Harden qa-eval-check against fork PRs, shell failures, and long descriptions

Four defensive changes, no behavior change on the happy path:

- Skip the job on fork PRs (GITHUB_TOKEN is read-only there, so the
  statuses POST 403s and produces a green run with no qa-eval context
  posted -- the same fork guard qa-eval-status.yml already has).
- `set -e` at the top of the shell script so a typo or failed `gh api`
  call fails the step instead of silently continuing.
- `grep '^skills/' ... || true` so an empty match exits 0 under `set -e`
  rather than tripping the shell -- the no-skills path then posts
  success and exits cleanly.
- Truncate the failure description to 140 chars before posting, since
  GitHub caps status descriptions at that length and a multi-skill
  failure would otherwise 422 the POST and lose the status entirely.

* Add called_successfully trajectory check (public mirror of harness a3d5167)

Pairs with 10gen/agent-skills-evals a3d5167, which added the
called_successfully enum value to the harness's evals.schema.json. This
is the public-repo half of the same change:

- testing/evals.schema.json: add called_successfully to the trajectory-
  check enum (and update the oneOf title). The public schema must agree
  with the harness schema, or a case validates in CI here and then fails
  -- or silently mis-grades -- in the harness.
- testing/mongodb-query-optimizer/evals/evals.json: flip the
  uses-mongodb-mcp check from "called" to "called_successfully". This is
  the exact check that would have caught the AXP false positive, where
  every mcp__mongodb__* call returned "No such tool available" yet
  attempt-counting (called) looked healthy. Prefer called_successfully
  for any gating check.

* ci(qa-eval): adopt PR #51's AWS Secrets Manager / OIDC custody for the Grove key

Replace the raw GitHub Actions secret (GROVE_API_KEY) with the
Application-Security-approved pattern from mongodb/agent-skills#51
(skill-gate, ENTSEC-5465): privileged jobs declare environment: qa-eval +
id-token: write, assume an AWS IAM role via OIDC, and fetch the Grove key
from AWS Secrets Manager at runtime. The key is never a GitHub secret; only
the role ARN (environment secret) + base URL (environment variable) live in
GitHub.

- qa-mutation-battery.yml: custody swap only (trusted content, no split).
- qa-pr-screen.yml: custody swap + collect/screen split mirroring PR #51.
  The untrusted PR-head checkout moves to an unprivileged `collect` job that
  stages content as an artifact; the privileged `screen` job downloads it and
  never checks out a PR ref, so OIDC assumption + key fetch never share a
  runner with untrusted content. (Caveat recorded in the header: the harness
  still executes against untrusted eval YAML with the key present, so this
  protects the credential-custody boundary but is not equivalent to
  skill-gate's data-only consumption.)
- qa-eval-check.yml / qa-eval-status.yml: trim stale "superseded if Evergreen
  lands" notes (Evergreen is not being pursued; fork limitation is permanent
  for this GHA path).

Triggers stay commented out — this makes the workflows ready to enable; it
does not enable them. Enabling requires the AWS IAM role + GitHub `qa-eval`
environment to be provisioned (separate infra step).

Co-Authored-By: Claude <noreply@anthropic.com>

* ci(qa-eval): fetch GitHub App PEM from AWS Secrets Manager; split publish env

Move the agent-skills-evals GitHub App private key out of GitHub secret storage
into AWS Secrets Manager (agent-skills/gh-app-private-key), fetched at runtime via
the same OIDC path as the Grove key. create-github-app-token now consumes the PEM
from env (GITHUB_APP_PRIVATE_KEY); App ID remains a repo secret (non-sensitive).
No long-lived credential lives in GitHub.

Split the mutation battery's `publish` job into its own `qa-eval-publish`
environment (no required-reviewers) so automated baseline publishes on
push/schedule never stall on approval, while the Grove-spend jobs (battery,
screen) stay on `qa-eval` where required-reviewers can be added later. Both
environments are in the IAM role's OIDC trust.

AWS-side prereqs (applied): role trust widened to skill-gate + qa-eval +
qa-eval-publish; role permissions grant GetSecretValue on agent-skills/grove*
and agent-skills/gh-app-private-key*; both SM secrets exist.

Co-Authored-By: Claude <noreply@anthropic.com>

* ci(qa-eval): replace report-commit gate with /qa-eval comment trigger + check runs

Reworks the QA-eval gate surface per the 2026-08-14 design decisions:

- qa-eval.yml (new): issue_comment `/qa-eval` trigger (OWNER/MEMBER only) +
  workflow_dispatch smoke path. Privilege split mirrors skill-gate.yml (PR #51,
  ENTSEC-5465): unprivileged `collect` does the data-only PR-head checkout and
  stages skills/ + testing/ as an artifact; privileged `eval` (environment:
  qa-eval, id-token) fetches the Grove key + App PEM from AWS Secrets Manager
  and runs the harness + scripts/ci_gate.py per changed skill. Verdict lands as
  the `qa-eval` check run (in_progress -> completed; INCONCLUSIVE maps to
  `neutral`), with per-skill JSON reports as artifacts and a short marker
  comment linking to the check run.
- qa-eval-status.yml: seeder now creates the `qa-eval` check run (queued /
  completed-success) instead of posting a commit status. Check runs are per-SHA
  by construction, so a stale pass can never satisfy a new head.
- qa-eval-check.yml: deleted. The report-commit + freshness-check design is
  superseded: the workflow posts onto the very SHA it evaluated, check runs
  work for fork PRs (base-repo context, no push to the fork branch), and no
  machine-generated commits land in git.

Kill switch: both workflows are inert until repo var QA_EVAL_ENABLED='true'
(flip after 10gen/agent-skills-evals#18 merges and a dispatch smoke test).
Known rollout verification: confirm `neutral` does not satisfy a required
`qa-eval` check under the ruleset (see qa-eval.yml header).

* fix(qa-eval): fetch AWS secrets before consuming GITHUB_APP_PRIVATE_KEY

In qa-pr-screen's screen job and qa-mutation-battery's battery job, the
create-github-app-token step consumed ${{ env.GITHUB_APP_PRIVATE_KEY }} four
steps BEFORE the aws-secretsmanager-get-secrets step that exports it (the
secret action writes to $GITHUB_ENV, visible to ${{ env.* }} in later steps
only). The expression resolved empty and the token step would have failed the
first time either trigger was uncommented — a leftover of the custody swap
(dd6eba5), which added the SM fetch but left its consumer above it. The
mutation battery's publish job already had the correct order; the two spend
jobs now match it.

* docs(qa-eval): make comments direct and current; drop historical narration

Comment- and description-only pass over the PR's files. Rationale comments
now state the principle ("counting attempts reads failed calls as healthy")
instead of narrating the incident that taught it, prerequisites are stated
as requirements rather than resolved/unresolved history, and pointers to
docs outside this repo are kept only where they name the code being run.
No behavior change: validator + tests + actionlint green locally.

Notably, the two dormant workflows' prereq lists were also stale after the
AWS-SM custody swap (they still listed AGENT_SKILLS_EVALS_APP_PRIVATE_KEY as
a GitHub secret to provision; the PEM comes from Secrets Manager now).

* docs(qa-eval): note the changed-files API ceiling in the seeder

The seeder is the one QA-gate workflow that lists changed files via the
pull-request files API (capped at 3,000 files/PR); the collect jobs diff
via git instead. Points at the PR-size expectations in CONTRIBUTING.md.

* Switch qa-eval.yml from AWS Secrets Manager to GHA secrets

GROVE_API_KEY → secrets.GROVE_API_KEY (qa-eval environment secret).
AGENT_SKILLS_EVALS_APP_PRIVATE_KEY → repo secret (read directly by
create-github-app-token, no longer via $GITHUB_ENV from the AWS step).

Removed: id-token:write, configure-aws-credentials,
aws-secretsmanager-get-secrets, AWS_ROLE_ARN prereq, the secrets-ordering
comment. The collect/eval split is preserved — secrets still never share a
runner with the untrusted checkout.

Companion to mongodb/agent-skills#51's identical switch.

* Switch qa-pr-screen.yml from AWS Secrets Manager to GHA secrets

GROVE_API_KEY → secrets.GROVE_API_KEY (qa-eval environment secret).
AGENT_SKILLS_EVALS_APP_PRIVATE_KEY → repo secret (read directly by
create-github-app-token). Removed id-token:write, configure-aws-credentials,
aws-secretsmanager-get-secrets, the secrets-ordering comment, and the
AWS_ROLE_ARN prereq guard. collect/screen split preserved.

Companion to mongodb/agent-skills#51's identical switch.

* Switch qa-mutation-battery.yml from AWS Secrets Manager to GHA secrets

GROVE_API_KEY → secrets.GROVE_API_KEY (qa-eval environment secret).
AGENT_SKILLS_EVALS_APP_PRIVATE_KEY → repo secret (read directly by
create-github-app-token). Removed id-token:write, configure-aws-credentials,
aws-secretsmanager-get-secrets, the secrets-ordering comment, and the
AWS_ROLE_ARN prereq guard from both the `battery` and `publish` jobs. The
`publish` job's qa-eval-publish environment no longer needs OIDC.

Companion to mongodb/agent-skills#51's identical switch.

* Merge collect+eval into a single job (review simplification)

Collapses the two-job collect/eval split into one `eval` job, mirroring the
review-driven simplification on skill-gate (#51): with GHA secrets the token's
only write scopes are pull-requests:write + checks:write (no contents:write),
and the split didn't protect the harness-executes-untrusted-content risk
anyway. The artifact upload/download round-trip is gone.

- Single `eval` job checks out the PR head (data-only, sparse, persist-
  credentials false) and runs the harness in-place; SKILL_BASE_DIR now points
  at $workspace/skills (checkout at root, no agent-skills/ staging path).
- Steps after the skill-list are gated on `steps.list.outputs.skills != '[]'`;
  finalize is `if: always() && … != '[]'` so a no-op /qa-eval leaves the
  seeder's check-run state standing instead of posting a spurious verdict.
- Security header rewritten to the single-job justification (CodeQL alert
  accepted, dismissed with header rationale) + the HONEST CAVEAT that the
  harness executes untrusted eval YAML with the key present.
- Removed "See setup doc step 3." (internal doc, not in repo).

qa-pr-screen.yml and qa-mutation-battery.yml stay two-job — they matrix over
a dynamically-computed skill list, which structurally requires a separate
list-finding job.

* Add concurrency to qa-eval-status seeder

Mirrors skill-gate-status.yml (PR #51): group by PR head sha,
cancel-in-progress: true. Safe because the trigger is `pull_request`
(not issue_comment), so only a new push to the same head supersedes —
an unrelated event never kills an in-flight seed.

* Revert unintended harness-flag change in merge

The merge commit accidentally changed the harness invocation from
`--epochs 1` to `--eval-kind qa` (and the echo from `k=1` to
`eval_kind=qa`). The merge was meant to be structure-only; restore the
original `--epochs 1` / `k=1` so the harness CLI contract is unchanged.

* refactor(qa-eval): simplify gate workflows + echo lint

- qa-pr-screen.yml: drop the fork-notice/collect jobs and artifact handoff —
  the pull_request trigger is commented out, so the credential-custody split
  was untestable dead weight. Dispatch-only now: one job, direct checkout of
  the operator-chosen (trusted) ref. The header documents that the split must
  be restored from history if a pull_request trigger is ever enabled.
- qa-mutation-battery.yml: drop the dead push-event diff branch and the
  per-workflow credential guard.
- Changed-skill resolution moves to .github/scripts/qa-resolve-skills.sh
  (same pattern as validate-skills.sh), derived from the GitHub files API —
  no more fetch-depth: 0 history clones just to git-diff. qa-eval.yml fetches
  the script from the DEFAULT branch at runtime rather than from its sparse
  PR-head checkout, preserving the rule that no PR-provided executable runs
  in the credential-bearing job.
- qa-eval.yml: drop the PR-comment upsert — the check run already renders
  the verdict, and dropping the comment path lets the job shed
  pull-requests: write (only checks: write remains). Verdict index is
  derived from the per-skill reports with jq -s instead of hand-assembled
  JSON. Finalize falls back to a check-run name lookup / create when
  CHECK_RUN_ID was never set, so a cancelled or timed-out run can no longer
  leave the (future) required check stuck in_progress blocking merge.
- lint-item-echo.mjs: drop the cross-skill null percentile leg. Measured on
  this corpus the null collapses to 0.000, so the leg could never change an
  outcome; minAbsoluteContainment + verbatim span + co-movement are the
  checks. echo-thresholds.json drops percentile and says so. The Python
  analysis layer (agent-skills-evals) makes the same removal.

* refactor(lint-item-echo): make the echo lint a true port of analysis/echo.py

The two implementations shared constants via echo-thresholds.json but not a
tokenizer: the lint split on non-alphanumerics and kept domain vocabulary,
echo.py kept hyphens/sigils and stripped it — so the shared 0.1 floor was
applied to differently-computed quantities, and the lint's verbatim-span
rule used a consecutive-n-gram-run approximation that could stitch a 'span'
from matches at different corpus locations.

Now a true port, verified numerically (containment/span outputs identical
on shared fixtures): same regex tokenizer with the same sigil/hyphen
normalisation, fenced-code blocks stripped on BOTH sides (an item restating
a code example is legitimate reuse, not echo), the stopword list promoted
into echo-thresholds.json as part of the shared contract, the exact
binary-search longest-common-run algorithm for the span, and corpus
construction aligned (sorted recursive references/**). The one deliberate
difference is reporting granularity: the lint scores prompt and answer
fields separately, the Python module pools them.

Recalibration on the corpus: the one prior finding (mcp-setup case 2, 11%
containment) goes away — it was carried by domain vocabulary, the false-
positive class the domain stopwords exist to suppress.

The lint is restructured to validate-evals.mjs's pattern (exports + main
guard) so lint-item-echo.test.mjs can pin the port; its fixtures and
expected values are identical to test_echo.py's — the two repos' CIs can't
import each other, so both pin the same numbers.

* fix(qa-pr-screen): route dispatch inputs through env, not run-block interpolation

Semgrep run-shell-injection findings on 4d89b52 (three threads, all this
file): github.event.inputs.skill and github.ref were interpolated directly
into run: scripts. Dispatch inputs are operator-only, but the env-var
indirection convention is what lets that safety not depend on who can
dispatch — and qa-eval.yml/qa-mutation-battery.yml already follow it. All
four steps now take SKILL (and REF) from the step env: block.

* Address Copilot review: align npm test with CI, reject directory files entries

- testing/package.json: npm test now runs lint-item-echo.test.mjs alongside
  validate-evals.test.mjs, matching the CI step that runs both.
- testing/validate-evals.mjs: checkAssetsExist rejects files entries that are
  directories. existsSync() alone passes them; the harness would only fail
  later with EISDIR. statSync follows symlinks, so a symlink to a directory
  is caught too. Pinned by a new test in validate-evals.test.mjs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Address review: enforce unique case ids in validate-evals

The schema documents ids as unique within a skill, but ajv cannot express
that (uniqueItems compares whole items) and the validator did not check it.
Duplicate ids merge unrelated cases into one identity in qa_runs metadata
and mutation analysis (see test_case_id_identity.py in agent-skills-evals).

Adds duplicateIds() alongside the other rules the schema cannot express,
pinned by two new tests.

* Address review: conclude failure, not neutral, on inconclusive qa-eval runs

GitHub rulesets treat a `neutral` check-run conclusion (like `skipped`) as
satisfying a required check, so an instrument fault (Grove 401, harness
crash, no reports) would have waved a PR through unmeasured. INCONCLUSIVE
now concludes `failure` with a "re-run needed" title to distinguish infra
from a real eval verdict. Blocking merge on inconclusive is the intended
behavior: a Grove outage blocks the gate until fixed.

* Address review: seed qa-eval status from the shared skill resolver

The seeder grepped only skills/** while qa-resolve-skills.sh also counts
testing/<skill>/**, so a tests/fixtures-only PR got an instant success and
merged without the suite it modified ever running. The seeder now sparse-
checks-out the PR head and runs the same resolver (fetched from the default
branch) as qa-eval.yml, so the two cannot disagree about what requires
evaluation.

* Address review: serialize mutation batteries globally, abort rebase before retry

The concurrency group keyed on the skill input, so an all-skills run and a
targeted run held different locks while publishing the same baseline file.
One global group now serializes all batteries.

The publish retry loop could also wedge: a rebase conflict leaves git
mid-rebase, where every further `git pull --rebase` fails on state rather
than content. The loop now aborts an active rebase and fails loudly on
conflict (a human picks which measurement stands), retrying only genuine
push races.

* Address review: fail the qa-eval check on partial skill coverage

An explicit `/qa-eval <skill>` or dispatch input evaluated only the named
skill, and the finalize step marked the check success regardless — so a
multi-skill PR could satisfy the required check with one (even unchanged)
skill evaluated. The finalize step now re-resolves the PR's changed skills
with the shared resolver and concludes failure ("partial coverage",
naming the missing skills) when the evaluated set doesn't cover them.
A resolver failure mid-finalize concludes failure too rather than leaving
the check wedged in_progress.

* Address review: pin qa-pr-screen dispatch to the PR's head SHA

pr_number previously controlled only where the result posted; the evaluated
code came from the operator-chosen dispatch ref, so a run of main could be
posted to a PR as though the PR had been screened. When pr_number is given
the workflow now resolves the PR's current head SHA, checks out that exact
SHA, stamps the evaluated SHA into the comment and job summary, and
re-resolves the head before posting — refusing to post if the PR moved
mid-run. Dispatches without pr_number still evaluate an arbitrary trusted
ref and print only.

* Migrate harness functional cases into public evals.json

The companion harness (10gen/agent-skills-evals) selected functional cases
by hard-coded private ids that did not exist in the public files, so the
QA_TESTING_DIR gate path found nothing to run. Migrate the functional cases
into the public corpus, keeping ids stable for qa_runs correlation:

- natural-language-querying: 37-48 (the routing-eval lift-positive core)
- query-optimizer: 37 (userId equality index). Private 38 is NOT migrated:
  it is the private twin of public case 20 (same ESR explain check).
- search-and-ai: 37

All five referenced seed fixtures already live in the public tree; the full
corpus (now 116 cases) validates clean. Field mapping: yaml target list ->
expectations; seed/functional_check/trajectory_checks carry verbatim.

* Address Copilot review: escape classification, error cap, Node version

- filesEntryEscapes no longer reports a ref resolving to the evals dir
  itself (files: ["."], rel === "") as an escape. It is contained but a
  directory, so it now falls through to the isFile() guard and reports
  "directory, not a file" — the classification that tells the author what
  to fix.
- Drop the slice(0, 10) cap on schema violations: the header promises EVERY
  violation so an author fixes the set in one pass, and the cap silently
  broke that contract for large breakage.
- validate-eval-cases.yml moves to Node 24, matching the repo's other Node
  workflows and tools/package.json engines (>=22.18).

* Address Copilot review: gate critical checks on MCP call success, not attempt

trajectory_checks marked critical: true used type: 'called', which only
asserts the MCP tool call was attempted and passes even if it errored.
Switch all six to 'called_successfully' so the gating checks require the
call to have returned without error, per the schema's own guidance.

* Address Copilot review: exact /qa-eval trigger + validator resilience

- qa-eval: require an exact '/qa-eval' or '/qa-eval <skill>' comment instead
  of the startsWith prefix, which also fired on e.g. '/qa-evaluate' and
  started the privileged eval job.
- qa-eval: short-circuit EVALUATED to [] when no report files exist instead
  of calling jq -s with an empty argument list (which reads stdin and can
  hang or behave inconsistently).
- validate-evals.mjs: guard doc.evals / ev.files with Array.isArray so a
  malformed case file is reported as a schema violation instead of a
  TypeError that stops the validator before it can print its errors. Both
  guards are pinned by new unit tests.

* Address Copilot review: report itself, never crash, on unresolvable real paths

realpathSync can throw (ELOOP from a symlink cycle, or a race where the
path disappears between the stat and the resolve). The validator's
stated contract is to report every violation and exit non-zero, not to
die mid-scan and swallow the remaining findings — so catch the throw and
report it as a per-case problem. Also resolve the real path once instead
of three times (the message re-resolved it), so the comparison and the
message can't disagree. Pinned by a new symlink-loop regression test.

* fix(qa): close evaluation gate coverage gaps

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(qa): handle validator races and manual outcomes

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
cory
2026-08-28 12:37:54 -04:00
committed by GitHub
parent 6ebabd86eb
commit 47cc46148f
27 changed files with 4209 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/scripts/qa-resolve-skills.sh
Executable
+122
View File
@@ -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
.github/workflows/qa-eval-manual-clear.yml
+98
View File
@@ -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
.github/workflows/qa-eval-status.yml
+130
View File
@@ -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/<name>/** or testing/<name>/** for a real skill -> queued
# ("run /qa-eval")
# - PR deletes or renames a skill (removed skills/<name>/** 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/<skill>/**: 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/<name>/\` 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/<name>/\` or \`testing/<name>/\` 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
.github/workflows/qa-eval.yml
+504
View File
@@ -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 <skill>`) 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 <skill>` 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 <skill>` (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" ]
.github/workflows/qa-mutation-battery.yml
+249
View File
@@ -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/<skill>/evals/mutation.yaml), runs the QA harness against baseline + every
# mutant, and commits the resulting sensitivity report to
# agent-skills-evals/baselines/<skill>-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/<skill>-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
.github/workflows/qa-pr-screen.yml
+218
View File
@@ -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="<!-- qa-screen-check:$SKILL -->"
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
.github/workflows/validate-eval-cases.yml
+59
View File
@@ -0,0 +1,59 @@
name: Validate Eval Cases
# Schema-validate testing/<skill>/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 }}"
CONTRIBUTING.md
+14
View File
@@ -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
testing/echo-thresholds.json
+131
View File
@@ -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."
}
}
testing/evals.schema.json
+417
View File
@@ -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/<skill>/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 <seed-name>.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/<name>/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/<seed>.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__<server>__<tool>)."
},
"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."
}
}
}
]
}
}
}
testing/lint-item-echo.mjs
+372
View File
@@ -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 <ref>`, 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 <ref>). 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();
testing/lint-item-echo.test.mjs
+108
View File
@@ -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);
});
testing/mongodb-natural-language-querying/evals/evals.json
+463
View File
@@ -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
}
}
]
}
]
}
testing/mongodb-natural-language-querying/evals/fixtures/seeded_movies.js
+19
View File
@@ -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())
testing/mongodb-natural-language-querying/evals/fixtures/seeded_nlq_lift.js
+123
View File
@@ -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())
testing/mongodb-natural-language-querying/evals/fixtures/seeded_sample.js
+44
View File
@@ -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(),
)
testing/mongodb-query-optimizer/evals/evals.json
+63
View File
@@ -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
}
}
]
}
]
}
testing/mongodb-query-optimizer/evals/fixtures/seeded_events.js
+28
View File
@@ -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,
)
testing/mongodb-query-optimizer/evals/mutation.yaml
+54
View File
@@ -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
testing/mongodb-schema-design/evals/mutation.yaml
+24
View File
@@ -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
testing/mongodb-search-and-ai/evals/evals.json
+38
View File
@@ -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
}
}
]
}
]
}
testing/mongodb-search-and-ai/evals/fixtures/seeded_movies_search.js
+63
View File
@@ -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')
}
testing/package-lock.json
Generated
+307
View File
@@ -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"
}
}
}
}
testing/package.json
+14
View File
@@ -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"
}
}
testing/qa-resolve-skills.test.sh
+101
View File
@@ -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/<name>/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 filename<TAB>status 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"
testing/validate-evals.mjs
+255
View File
@@ -0,0 +1,255 @@
#!/usr/bin/env node
/**
* Validate every testing/<skill>/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/<name>/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/<name>/ 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();
testing/validate-evals.test.mjs
+191
View File
@@ -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/<name>/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");
});