test: scenarios check a skill's outcome against its contract (ADR 0012)

AGENTS.md

@@ -35,6 +35,8 @@ skills/<bucket>/<skill>/SKILL.md     exactly three levels deep
 
 **Every pipeline skill opens with a `## Contract`**: a YAML block that says what the skill is as a function (preconditions, the closed set of results, postconditions, side effects by scope, forbidden effects). **Process** says how. Change a result line, a precondition or an effect in the contract first. `agent-workflow-protocol` defines the keys (**Contracts**), `docs/adr/0010-skills-as-functions-with-contracts.md` explains why, and `npm run validate` checks that a contract matches the rest of its skill: `params` equal the Inputs table, every result the text returns is in `returns`, and there are no `Eligibility`, `Outputs`, `Result` or `Never` sections left over. Anything not listed under `effects.writes` is forbidden.
 
+**Every result a skill can return has a scenario** under `tests/scenarios/<skill>/`: a world, the inputs, the expected result line and the net effects. `scripts/scenario.mjs` checks each against the skill's contract (scopes, invariants, eligibility oracles), and `npm run scenarios:coverage` lists results with none. A new result line without a scenario fails `npm test`. See `docs/adr/0012-scenarios.md`.
+
 ## Inputs and defaults
 
 **Every skill has an `## Inputs` section.** It names the inputs the skill accepts, in two interchangeable forms: **key-value pairs** (`key=value` on a line of its own) and **free text** (whatever the prompt says). It also names the skill's **implicit dependencies**: the tools, skills and repository state it needs that are not inputs. Every pipeline skill lists `runner` and `unattended`, and every skill that reads markers lists `trusted_authors`.

docs/adr/0012-scenarios.md

@@ -0,0 +1,38 @@
+# 0012. Scenarios check a skill's outcome against its contract
+
+## Status
+
+Accepted
+
+## Context
+
+ADR 0010 gave every pipeline skill a contract, and ADR 0011 closed the gaps it exposed. A contract says what a skill promises. Nothing yet checked an outcome against it. The skills are instructions a model follows, so a test cannot run one without a model, and no model is available in CI.
+
+## Decision
+
+A **scenario** is a world, a skill and its inputs, and the expected outcome: one result line, and the net effects the run leaves behind (`tests/scenarios/<skill>/*.yaml`). `scripts/scenario.mjs` checks a scenario against the skill's real contract, read from its `SKILL.md`:
+
+1. The result is one of the contract's `returns`, and is about the target.
+2. Every effect is in a scope that `effects.writes` allows (`claimed`, `repository`, `other`, `shared`).
+3. The world after the effects satisfies the invariants:
+   - one state label per issue, one PR state label per PR, one PR holding the merge lane;
+   - the lease label is gone from the claimed item;
+   - no keep-open issue is closed;
+   - a merge is pinned to the reviewed head, is not in a run that pushed, and is not of a fork, a draft, an `agent:stuck` PR or a protected path;
+   - `skipped` leaves no effects, `noop` leaves only the lease comment, `escalated` adds `agent:stuck` with an `agent:escalate` comment, `failed` leaves the labels alone;
+   - a dry run writes nothing, `limit` is respected, and only skills in the contract's `calls` are called.
+4. Eligibility **oracles**, which are `requires` written as code, agree with the result. `shepherd-github-pr` has one.
+
+`tests/scenarios.test.mjs` runs every scenario, runs negative tests that prove the checker rejects broken ones, and requires every result a skill declares to have at least one scenario (`npm run scenarios:coverage`).
+
+## Consequences
+
+- A new result line, or a changed write scope, cannot land without a scenario. The old behaviour of the gap-9 bug (a conflicted PR skipped as `no-checks-yet`) is now a failing test.
+- The same checker can judge a real run. Diff the world before and after, turn the diff into effects, and pass the result line. The part that is not built is the part that needs infrastructure: a `gh` simulator over a world file, and a runner that drives a model against it.
+
+## What this does not prove
+
+- **It does not run a skill.** The goldens are written from the skill text, so they check that the contract, the expected outcomes and the invariants agree with each other. They cannot show that a model follows the text.
+- **The goldens can be wrong in the same way as the contract.** Both came from the same reading of the skills. A scenario that a real run contradicts should change one of them.
+- **Scopes are checked, not the lists under them.** The contract's `effects.writes` entries are prose, so an effect is checked against its scope and not against the exact sentence.
+- **Only one eligibility oracle exists.** Writing `requires` as code for the other skills duplicates the prose, so each one is a judgement about whether the check is worth keeping in step.

package.json

@@ -8,7 +8,7 @@
     "validate": "node scripts/validate-skills.mjs",
     "lint": "node scripts/lint-files.mjs --all",
     "pretest": "node scripts/check-hooks.mjs",
-    "test": "bash tests/lib.test.sh && node --test tests/helpers.test.mjs tests/lint.test.mjs tests/contracts.test.mjs",
+    "test": "bash tests/lib.test.sh && node --test tests/helpers.test.mjs tests/lint.test.mjs tests/contracts.test.mjs tests/scenarios.test.mjs",
     "check-plugin-version": "node scripts/sync-plugin-version.mjs --check",
     "check": "npm run lint && npm run validate && npm run check-plugin-version && npm test",
     "hooks:check": "node scripts/check-hooks.mjs",
@@ -17,7 +17,8 @@
     "changeset": "changeset",
     "version": "changeset version && node scripts/sync-plugin-version.mjs",
     "sync": "node scripts/sync.mjs",
-    "sync:check": "node scripts/sync.mjs --check"
+    "sync:check": "node scripts/sync.mjs --check",
+    "scenarios:coverage": "node scripts/scenario-coverage.mjs"
   },
   "dependencies": {
     "mattpocock-skills": "1.3.0"

scripts/scenario-coverage.mjs

@@ -0,0 +1,40 @@
+// Reports which of each skill's declared results no scenario exercises.
+//   npm run scenarios:coverage
+// The same numbers are enforced by tests/scenarios.test.mjs: a skill cannot declare a result line
+// that no scenario exercises.
+import { readFileSync, readdirSync, existsSync } from "node:fs";
+import { join, dirname } from "node:path";
+import { fileURLToPath, pathToFileURL } from "node:url";
+import { parse } from "yaml";
+import { extractContract } from "./contracts.mjs";
+import { matchesReturn } from "./scenario.mjs";
+
+const PKG = join(dirname(fileURLToPath(import.meta.url)), "..");
+const skills = readdirSync(join(PKG, "skills/pipeline")).filter((n) => existsSync(join(PKG, "skills/pipeline", n, "SKILL.md"))).sort();
+/** @returns {{name: string, total: number, missing: string[]}[]} one row per non-group pipeline skill */
+export function coverage() {
+  const rows = [];
+  for (const name of skills) {
+    const c = parse(extractContract(readFileSync(join(PKG, "skills/pipeline", name, "SKILL.md"), "utf8")).yaml);
+    if (c.kind === "group") continue; // a group's results are its items' results
+    const dir = join(PKG, "tests/scenarios", name);
+    const results = existsSync(dir)
+      ? readdirSync(dir).filter((f) => f.endsWith(".yaml")).flatMap((f) => {
+          const sc = parse(readFileSync(join(dir, f), "utf8"));
+          return (sc.expect.results ?? [sc.expect.result]).flatMap((r) => [r, r.replace(/^would: /, "")]);
+        })
+      : [];
+    rows.push({ name, total: c.returns.length, missing: c.returns.filter((r) => !results.some((x) => matchesReturn(x, [r]))) });
+  }
+  return rows;
+}
+
+if (import.meta.url === pathToFileURL(process.argv[1] ?? "").href) {
+  let total = 0, covered = 0;
+  for (const { name, total: t, missing } of coverage()) {
+    total += t; covered += t - missing.length;
+    console.log(`${name.padEnd(26)} ${String(t - missing.length).padStart(2)}/${t} results covered`);
+    for (const m of missing) console.log(`    not covered: ${m}`);
+  }
+  console.log(`\n${covered}/${total} declared results have a scenario`);
+}