mirror of
https://github.com/mongodb/agent-skills.git
synced 2026-09-18 21:15:11 +08:00
cory
47cc46148f
* 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>
11 KiB
11 KiB
Contributing
Repository layout
This repo publishes two plugins from plugins/:
-
plugins/mongodb/— the community plugin (localnpxMCP server). -
plugins/mongodb-atlas/— the Atlas plugin (MongoDB-hosted HTTP MCP server). Its OAuth client ID and config syntax differ per client, so each client'splugin.jsoninlines its ownmcpServersconfig:.claude-plugin/plugin.json— Claude (oauth.clientId, a CIMD URL).cursor-plugin/plugin.json— Cursor (auth.CLIENT_ID).grok-plugin/plugin.json— Grok (oauth.clientId).github/plugin/plugin.json— GitHub Copilot / VS Code (oauth.clientId, a CIMD URL)- On Codex it points at a connector via
.app.json.
The pre-registered client IDs the Atlas auth server accepts are defined in the
atlas-iamOAuth client seed (private; MongoDB employees only).
Each plugin is published to several ecosystems, and each looks for its catalog
index and per-plugin manifest in a different location. The same skills/ content
backs all of them:
| Ecosystem | Plugins listed | Marketplace index | Per-plugin manifest |
|---|---|---|---|
| Claude | mongodb, mongodb-atlas |
.claude-plugin/marketplace.json |
plugins/<name>/.claude-plugin/plugin.json |
| Cursor | mongodb, mongodb-atlas |
.cursor-plugin/marketplace.json |
plugins/<name>/.cursor-plugin/plugin.json |
| Codex | mongodb, mongodb-atlas |
.agents/plugins/marketplace.json |
plugins/<name>/.codex-plugin/plugin.json |
| Grok (xAI) | mongodb, mongodb-atlas |
.grok-plugin/marketplace.json |
plugins/<name>/.grok-plugin/plugin.json |
| Copilot / VS Code | mongodb-atlas only |
.github/plugin/marketplace.json |
plugins/<name>/.github/plugin/plugin.json |
| Antigravity | mongodb, mongodb-atlas |
(none) | plugins/<name>/.agy-plugin/plugin.json |
The plugin format is shared across Grok, Copilot, VS Code, and Claude —
Copilot/VS Code even accept .claude-plugin/ files as a fallback, so the
dedicated .github/plugin/marketplace.json is what keeps Copilot scoped to
mongodb-atlas only.
Antigravity has no plugin marketplace. It discovers the MCP config
and skills by folder convention — .agy-plugin/mcp_config.json
and .agy-plugin/skills/ — rather than the mcpServers/skills
manifest fields the other clients use. Because it treats
.agy-plugin/ as the plugin root, that folder needs its own
physical skills/ copy.
The repo-root skills/ directory is the canonical source of truth for all skill
content. Each plugin ships its own committed copy under plugins/<name>/skills/,
generated by tools/sync-plugin-skills.ts (which declares each plugin's skill
selection inline). For plugins that also have an .agy-plugin/ directory, the
script additionally regenerates plugins/<name>/.agy-plugin/skills/ with the
same selection, since Antigravity resolves skills relative to that folder. After
changing anything under skills/ or assets/, run:
node tools/sync-plugin-skills.ts
(Node 24 — or any Node ≥ 22.18 — runs the TypeScript directly, no transpile step.)
Commit the regenerated copies. CI (the Plugin Skills Sync workflow) fails if
they drift. The repo-root gemini-extension.json remains the community Gemini
extension; the Atlas plugin is not published on Gemini yet.
Pull requests
Use a branch-based workflow, not forks. CI gates on skill content (the
skill-gate security scan) run on demand via a maintainer comment (/skill-gate)
so model/API spend stays deliberate. On a fork PR the automatic status seeding can't
run (fork tokens are read-only) and the scan needs the repo's protected environment —
so a fork PR merges only after a maintainer triggers the gate for it. Pushing a branch
directly avoids all of that.
Keep PRs well under 3,000 changed files. The gates discover changed skills via 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
workflows rather than editing version fields, creating tags, or drafting GitHub
releases by hand. The version logic lives in tools/plugin-version.ts (the
workflows call it); you can run the same checks locally — node tools/plugin-version.ts check
validates that all plugin manifests share one version, and
node tools/plugin-version.ts bump <patch|minor|major|exact> applies a bump (run
pnpm install in tools/ first; bump uses the semver dependency).
- Start the
Prepare Releaseworkflow (.github/workflows/prepare-release.yml) from the GitHub Actions UI, usingmainas the workflow branch. - Choose the version source:
patch,minor, ormajorincrements the current plugin version.exactuses theexact_versioninput, for example1.2.3.
- Review the release PR that the workflow opens. Confirm that the target version is correct and that CI passes.
- Merge the release PR.
- Wait for the
Draft Releaseworkflow to create the draft GitHub release. - Review the generated notes, add any platform-specific context that should be
included, and confirm that the draft release tag matches the release PR
version and targets the merged
maincommit. Then publish the draft release. - Manually submit plugin updates for Cursor, Claude, Grok (xAI), and Github Copilot.
- Coordinate the VS Code extension update.
The repo release is complete after the GitHub release is published and the Cursor and Claude updates have been submitted. Marketplace approval, marketplace indexing, and downstream extension releases can happen afterward.
The Draft Release workflow also supports manual dispatch. If a release already
exists for the release tag, the workflow skips release creation.
Releasing individual plugins
| Plugin or surface | Visible to users when | Operator action |
|---|---|---|
| Gemini | The extension gallery picks up the latest GitHub release | None |
| Cursor | The Cursor Marketplace publishes the submitted update | Submit the update form |
| Claude | The Claude plugin directory publishes the submitted update | Submit the update form |
| Copilot (CLI / VS Code agent plugins) | Custom marketplace: changes merged to main. Official marketplace: after the awesome-copilot PR is merged |
Open or refresh the github/awesome-copilot PR (and bump the ref/sha) |
| Codex | Changes are merged to main |
None |
| Grok (xAI) | Custom marketplace: changes merged to main. Official catalog: after the upstream PR is merged |
Open or refresh the xai-org/plugin-marketplace PR (and bump the commit SHA) |
| VS Code extension | The VS Code extension publishes a release with the updated skills | Coordinate the extension update |
Gemini
No manual submission is required after publishing the GitHub release.
This repo is listed in the
Gemini extension gallery via
the gemini-cli-extension GitHub topic and root gemini-extension.json. The
gallery crawls tagged repositories daily and should pick up the latest GitHub
release automatically.
Cursor
After publishing the GitHub release, submit the update through the
Cursor Marketplace publish form. Use
the release tag as the submitted version and include any release notes that
matter to Cursor users. Submit both the mongodb and mongodb-atlas plugins. See the
plugin submission details
for what to include in the form. If you cannot access that document, stop and
ask for the required submission details.
Claude
After publishing the GitHub release, submit the update through the
Claude.ai form. Each update is
scanned before it is shared in the plugin directory. Submit both the mongodb and
mongodb-atlas plugins. See the
plugin submission details
for what to include in the form. If you cannot access that document, stop and
ask for the required submission details.
GitHub Copilot (CLI and VS Code agent plugins)
After publishing the GitHub release, open a PR adding mongodb-atlas to
github/awesome-copilot's
.github/plugin/marketplace.json, using a github source pinned to the release
commit:
{ "source": "github", "repo": "mongodb/agent-skills", "path": "plugins/mongodb-atlas", "ref": "<release-tag>", "sha": "<commit-sha>" }
Each update bumps the ref/sha. Only mongodb-atlas is published to Copilot,
not the community mongodb plugin.
Codex
No manual submission is required after the release PR is merged.
This repo's Codex marketplace is added from the GitHub repository at
ref: "main", and its plugin entries use local paths relative to that checked
out marketplace root. Codex does not have a native "latest GitHub release"
source selector.
Grok (xAI)
After publishing the GitHub release, open a PR adding the plugins to
xai-org/plugin-marketplace's
.grok-plugin/marketplace.json, pinned to the release commit SHA, and regenerate
its plugin-index.json.
VS Code extension
The VS Code extension is released from the
mongodb-js/vscode repository. To include
skill changes from this repo, update the extension's agent-skills submodule to
the latest released tag, then run pnpm run sync-skills in the extension repo
to pull in any skill changes before releasing the extension.