diff --git a/.claude/skills/testing/SKILL.md b/.claude/skills/testing/SKILL.md index 7a597e4c..56553ed0 100644 --- a/.claude/skills/testing/SKILL.md +++ b/.claude/skills/testing/SKILL.md @@ -11,11 +11,18 @@ statements, functions, and branches.** That rule and the React/Mantine conventions live in [`AGENTS.md`](../../../AGENTS.md); this skill is where a test goes, how to run it, and how to clear the gate. -⚠️ **Anything that needs a real server to run against — an integration test, a -smoke, reproducing a bug by hand — is `/test-servers`, and you have to load it.** -Integration and smoke tests here drive a real server over a real transport -rather than a mock, so picking, building and connecting to a fixture is a -procedure of its own that this skill does not carry. +## 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. + +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. ## Where the test file goes @@ -38,7 +45,9 @@ web-owned test living under `src/test/` instead is a bug. `core/` source layout (`mcp/`, `mcp/node/`, `mcp/remote/`, `auth/`, `auth/node/`, `storage/`). **Placement is the manifest** — any file under that folder is picked up by the integration project (node env, 30s timeouts) via a - folder glob; there is no enumeration to keep in sync. + folder glob; there is no enumeration to keep in sync. ⚠️ These run against a + real server, so **load the `test-servers` skill before writing one** — the + fixture is half the test. 3. **Shared test infrastructure** — `renderWithMantine.tsx`, `setup.ts`, `fixtures/`, `scrollAreaStoryAssertions.ts`. @@ -82,6 +91,10 @@ 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. + `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 inside the `coverage` gate. CI therefore has no separate `test:integration` step. @@ -159,5 +172,8 @@ shared helper that wraps one. ## Test servers, not mocks -Integration and smoke tests drive a real server over a real transport. See -`/test-servers` for picking and building one. +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. diff --git a/docs/skill-authoring.md b/docs/skill-authoring.md index 6e515833..5692f3b1 100644 --- a/docs/skill-authoring.md +++ b/docs/skill-authoring.md @@ -289,13 +289,25 @@ 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. -**The committed cases have measured 33% / 33% on one `RUNS=3` run and 100% / -33% on another, and at least one of them being red is the intended state rather -than an oversight.** `skills:eval` is not a gate (see below), and the number is -the finding: `testing` points at `test-servers` in its first paragraph and the -model follows that pointer *sometimes*. Strengthening it is its own change -against its own issue (#2247); lowering the bar to turn the column green would -throw away the only signal this feature adds. +**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 +`testing` -> `test-servers` cases are the worked example. They measured +33% / 33% at `RUNS=3` when `testing` opened with a one-sentence ⚠️ *classifying* +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%. + +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 +lever on a *description*'s shape; this is it applied to a body. The corollary is +where to put one: the top of a body is read before the model knows it needs the +second skill, so a pointer that lives only there is a pointer it has already +scrolled past by the time it matters. ⚠️ **Do not read a rise between two `RUNS=3` runs as an improvement.** One sample is 33 points there, and the two runs above straddle a 67-point swing on @@ -372,7 +384,7 @@ The summary is two lines, never one: ``` 7/7 first-move cases at or above 80%. -1/2 hand-off cases above 50%. +2/2 hand-off cases above 50%. ``` Narrowing the run never narrows what a **negative** case is scored against — a diff --git a/scripts/skill-eval.mjs b/scripts/skill-eval.mjs index 748cdaba..1fc38b6c 100755 --- a/scripts/skill-eval.mjs +++ b/scripts/skill-eval.mjs @@ -57,10 +57,13 @@ const THRESHOLD = Number(process.env.THRESHOLD ?? 0.8); // 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 measure 33% (RUNS=3) against a -// pointer that is live and stated in the first paragraph of `testing`'s body, -// so an 0.8 bar would mark every hand-off red regardless of how strongly the -// first skill points at the second, and the column would stop carrying signal. +// 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.) // // 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