diff --git a/.claude/skills/testing/SKILL.md b/.claude/skills/testing/SKILL.md index 56553ed0..c1683d8c 100644 --- a/.claude/skills/testing/SKILL.md +++ b/.claude/skills/testing/SKILL.md @@ -14,15 +14,22 @@ test goes, how to run it, and how to clear the gate. ## Before you write it: does the test need a real server? **If it does, load the `test-servers` skill now — that is step one, before -choosing a location or writing a line.** It does, whenever the task is: an -integration test; an end-to-end test; a smoke; a coverage gap that has to be -exercised over a transport; or reproducing a reported bug by hand. +choosing a location or writing a line.** -Every one of those drives a **real server over a real transport, never a mock**, -and picking the fixture, building it, and connecting with the right protocol era -is a procedure this skill does not carry. Writing one without `test-servers` -means hand-rolling a fixture that already exists, or mocking the thing the tier -exists to avoid mocking. +The condition is **"does this exercise MCP behaviour over a transport?"**, not +which tier the test lands in. It does whenever the task is: an integration test; +an end-to-end test that connects; a smoke that drives a **connected** flow; a +coverage gap only reachable over a real connection; or reproducing a reported +bug against a server. It does **not** for a test that renders a component from +fixture props, or for a boot-only smoke that never connects — `smoke:launcher` +checks `--help` and `smoke:web` / `smoke:web:browser` only assert the SPA is +served and paints. + +When it does apply, the test drives a **real server over a real transport, never +a mock**, and picking the fixture, building it, and connecting with the right +protocol era is a procedure this skill does not carry. Writing one without +`test-servers` means hand-rolling a fixture that already exists, or mocking the +thing the tier exists to avoid mocking. ## Where the test file goes @@ -91,9 +98,14 @@ spawns the built binary) → smokes through the built launcher (`npm run smoke`) Storybook play functions (`test:storybook`) → the published-tarball check (`npm run pack:verify`, local/release only — needs network). -Everything from **web integration** rightwards needs a fixture from -`test-servers/` — load the `test-servers` skill as soon as a task puts you at -that tier or deeper. +⚠️ **Depth in that list is not the fixture boundary — connecting is.** Web +integration, the out-of-process CLI tests, the smokes that actually connect +(`smoke:cli`, `smoke:web:app`, `smoke:web:elicit`, `smoke:web:tabs`) and +`pack:verify` all need a fixture from `test-servers/`; `smoke:launcher`, +`smoke:web`, `smoke:web:browser` and every Storybook play function do not — the +first three stop at boot, and play functions render from fixture props. **Load +the `test-servers` skill as soon as a task puts you on the connecting side of +that line.** `validate` runs the per-client `test` scripts — so web **unit** plus cli's out-of-process `e2e.test.ts`, but **not** web's integration project, which runs @@ -172,8 +184,9 @@ shared helper that wraps one. ## Test servers, not mocks -Integration and smoke tests drive a real server over a real transport, never a -mock. **Load the `test-servers` skill to pick, build and run the fixture** — -which showcase config covers the feature, which protocol era to connect with, -how to add a combination that does not exist yet, and why a fixture can keep -serving stale code after an edit. +Integration tests, and the smokes that drive a connected flow, use a real server +over a real transport rather than a mock. **For those, load the `test-servers` +skill to pick, build and run the fixture** — which showcase config covers the +feature, which protocol era to connect with, how to add a combination that does +not exist yet, and why a fixture can keep serving stale code after an edit. A +boot-only smoke needs none of it (see the tier list above). diff --git a/docs/skill-authoring.md b/docs/skill-authoring.md index 5692f3b1..8acc4e88 100644 --- a/docs/skill-authoring.md +++ b/docs/skill-authoring.md @@ -284,10 +284,14 @@ pass 2/4 whenever `RUNS` is even, reporting a result the criterion does not license (Copilot). A strict bound of `1.0` is therefore unreachable and the harness rejects it up front rather than failing every case. -At 0.8 a hand-off case would be red no matter how strongly the first skill -pointed at the second, and the column would stop carrying signal. Read a -hand-off number as a description-strength measurement, not a verdict — and read -it at `RUNS=5`, since at `RUNS=3` one sample is worth 33 points. +0.5 is a floor on **useful reliability for a second-hop load**, not a claim that +0.8 is out of reach — a well-shaped pointer does clear it, as the worked example +below records. What 0.5 buys is that the column keeps carrying signal across the +*range* of pointer strengths a repo actually has: a hand-off is a noisier +measurement than a first move, so a bar set where a strong pointer sits marks +every merely-adequate one red and stops distinguishing them from a broken one. +Read a hand-off number as a description-strength measurement, not a verdict — +and read it at `RUNS=5`, since at `RUNS=3` one sample is worth 33 points. **A red hand-off case is a finding about the pointer, not a build break — and the fix is to reshape the pointer, never to lower the bar.** The committed @@ -296,11 +300,12 @@ the fix is to reshape the pointer, never to lower the bar.** The committed which work belongs to `test-servers`; rewriting that into an imperative first step ("load the `test-servers` skill now — that is step one"), and repeating it at the two later points where the model actually decides it is writing an -integration test, took them to **100% / 80% at `RUNS=5`** with the prompts -unchanged (#2247) — 100% / 100% on a focused `-- test-servers` run of the same -build, which is the size of the run-to-run noise still present at `RUNS=5`. -Nothing else moved: the two descriptions were not touched, and the same suite -scored **63/63** first-move cases at 100%. +integration test, took them to **100% / 100% at `RUNS=5`** with the prompts +unchanged (#2247). Nothing else moved: the two descriptions were not touched, +and the same suite scored **63/63** first-move cases at 100%. (An intermediate +build measured 100% / 80% on the full suite and 100% / 100% on a focused +`-- test-servers` run — worth knowing as the size of the run-to-run noise still +present at `RUNS=5`.) The transferable part is that **a pointer is followed when it reads as an action with a trigger, and skimmed when it reads as a fact.** #2202 found the same diff --git a/scripts/skill-eval.mjs b/scripts/skill-eval.mjs index 1fc38b6c..7b98e6da 100755 --- a/scripts/skill-eval.mjs +++ b/scripts/skill-eval.mjs @@ -56,14 +56,16 @@ const THRESHOLD = Number(process.env.THRESHOLD ?? 0.8); // A hand-off is a harder thing to hit than a first move, and what counts as // acceptable is a separate judgement rather than one inherited from a number // tuned for the other measurement. 0.5 is the weakest claim worth asserting — -// the pointer is taken more often than not. It is deliberately not 0.8: the -// committed `testing` -> `test-servers` cases measured 33% (RUNS=3) against a -// pointer that was live and stated in the first paragraph of `testing`'s body, -// so an 0.8 bar would mark a hand-off red regardless of how strongly the first -// skill points at the second, and the column would stop carrying signal. -// (#2247 later reshaped that pointer into an imperative step and took the same -// two cases to 100% at RUNS=5 — which raises the ceiling those cases reach, not -// the floor a *new* hand-off case should be judged against.) +// the pointer is taken more often than not — and it is a floor on useful +// reliability for a SECOND-HOP load, not a claim that 0.8 is unreachable. A +// well-shaped pointer does clear 0.8: the committed `testing` -> `test-servers` +// cases measured 33% (RUNS=3) when `testing` merely classified which work +// belonged to `test-servers`, and 100%/80% (RUNS=5) once #2247 reshaped that +// into an imperative step. What 0.5 buys is a column that still separates an +// adequate pointer from a broken one — a hand-off is a noisier measurement than +// a first move, so a bar set where a STRONG pointer sits would mark both red. +// A reshaped pointer therefore raises the ceiling those cases reach, not the +// floor a *new* hand-off case should be judged against. // // It is compared STRICTLY, unlike the first-move threshold. "More often than // not" is `> 0.5`, and an inclusive compare passes exactly half the samples