Test-first: acceptance tests in issues and skills

.claude/skills/add-feature/SKILL.md

@@ -6,11 +6,25 @@ argument-hint: [feature-name]
 
 # Add Feature
 
-Order of operations only; CLAUDE.md covers package boundaries, `.claude/rules/web-app.md` mutation architecture and
+Order of operations only, spec first; CLAUDE.md covers package boundaries, `.claude/rules/web-app.md` mutation architecture and
 `.claude/rules/icons.md` icon rules. Paths below are relative to `apps/web/src`, except repo-rooted ones, which begin
 with `apps/`, `packages/`, `scripts/` or `.github/`.
 
-## 1. Feature directory
+## 1. Failing spec first
+
+Start from the issue's **Acceptance tests** list and write one `it()` per behaviour before any feature code. Run it and confirm it fails **for the right reason**: an assertion about the missing behaviour, not an import error, a missing mock or a typo. When the subject file does not exist yet, add a stub export (a component returning `null`, a function returning `undefined`) so the run reaches the assertion. Steps 2 to 7 then make it green. If the issue says `No test-first: <reason>` (pure layout or CSS, a dependency upgrade, infra config), skip this step and repeat that line in the PR.
+
+Vitest's `include`, set in `apps/web/vitest.config.mts` and relative to `apps/web/`, is
+`src/specs/**/*.spec.{ts,tsx}`, so a spec beside the component is not collected. Put it in
+`specs/features/<feature-name>/`, render with `renderWithQueryClient` from
+`@/specs/test-utils` and seed with `seedQueryClient(queryClient, data)`, client first.
+i18next is globally mocked to return the key, so assert on `"<feature-name>.button-label"`.
+The same global setup (`specs/setup-any-spec.ts`) mocks `@/utils` down to `random` and
+`getAccessToken`, so a spec that reaches a feature-local api wrapper pulling anything else
+from `@/utils` has to re-mock it with `vi.importActual("@/utils")`. `pnpm test` at the root
+is web-only.
+
+## 2. Feature directory
 
 Files sit flat at the feature root with an `index.ts` barrel. Most feature dirs have that
 barrel; a few carry `components/`, `hooks/`, `api/` or `types/` subfolders. Do not scaffold
@@ -31,7 +45,7 @@ here. A feature stylesheet Tailwind cannot express is registered as an `@import`
 `styles/_shared-components.scss`, which `styles/style.scss` pulls in once. Do not import it
 from the component.
 
-## 2. Flag (only if toggleable)
+## 3. Flag (only if toggleable)
 
 Add to `visionFeatures` in `config/config.ts`, then mirror into `config/config.template.ts`
 by hand: nothing imports the template, so a missed edit is not caught. Gate with
@@ -45,7 +59,7 @@ by hand: nothing imports the template, so a missed edit is not caught. Gate with
 
 Outside JSX: `getConfigValue(({ visionFeatures }) => ...)`.
 
-## 3. Data
+## 4. Data
 
 - Blockchain: run `/add-sdk-mutation` or `/add-query` first.
 - Private API: the client is `packages/sdk/src/modules/private-api`. There is no
@@ -57,7 +71,7 @@ Outside JSX: `getConfigValue(({ visionFeatures }) => ...)`.
 - `useActiveAccount()` from `@/core/hooks/use-active-account`. Keys: `QueryKeys` from
   `@ecency/sdk`, or `QueryIdentifiers` from `@/core/react-query` for web-only queries.
 
-## 4. i18n
+## 5. i18n
 
 The app imports the i18next default export directly. `react-i18next` and `useTranslation`
 are not used here.
@@ -75,33 +89,21 @@ one object is silently last-wins, guarded by
 `specs/features/i18n-locale-duplicate-keys.spec.ts`. Inline markup uses `Tsx` from
 `@/features/i18n/helper`.
 
-## 5. Page route (if it owns a URL)
+## 6. Page route (if it owns a URL)
 
 Plain `apps/web/src/app/<route>/page.tsx`. `(dynamicPages)` holds profile, entry, feed and
 community; other routes sit at the app root or under `(staticPages)`. Page-local components
 go in `_components/` beside `page.tsx`. `routes.ts` is a legacy map read by a few files; add
 to it for pattern matching.
 
-## 6. Specs
-
-Vitest's `include`, set in `apps/web/vitest.config.mts` and relative to `apps/web/`, is
-`src/specs/**/*.spec.{ts,tsx}`, so a spec beside the component is not collected. Put it in
-`specs/features/<feature-name>/`, render with `renderWithQueryClient` from
-`@/specs/test-utils` and seed with `seedQueryClient(queryClient, data)`, client first.
-i18next is globally mocked to return the key, so assert on `"<feature-name>.button-label"`.
-The same global setup (`specs/setup-any-spec.ts`) mocks `@/utils` down to `random` and
-`getAccessToken`, so a spec that reaches a feature-local api wrapper pulling anything else
-from `@/utils` has to re-mock it with `vi.importActual("@/utils")`. `pnpm test` at the root
-is web-only.
-
 ## 7. Wiring in
 
 - Toolbar button: `features/shared/editor-toolbar/index.tsx`, inside a `Conditional`. The
   publish route's `app/publish/_editor-extensions/` is not this.
 - Profile section: `app/(dynamicPages)/profile/[username]/_components/`.
 - Navigation: `features/shared/navbar/navbar-main-sidebar.tsx`.
 
-Finish with `pnpm lint`, `pnpm typecheck` and `pnpm test`. Those three run none of the script
+Before the PR, the spec from step 1 must be green and the PR body lists each spec path, the command and the assertion that failed before the change, then the green run after it. When step 1 was skipped, the PR repeats the issue's `No test-first: <reason>` instead. Finish with `pnpm lint`, `pnpm typecheck` and `pnpm test`. Those three run none of the script
 audits in `.github/workflows/typecheck.yml`; the ones a feature is likely to trip are
 `node scripts/icon-scss-audit.mjs`, `node scripts/icon-tsx-audit.mjs --fail` and
 `node scripts/slim-entries-audit.mjs --fail`. Copy each line's flags exactly. The two

.claude/skills/add-query/SKILL.md

@@ -23,15 +23,36 @@ an SDK file plus one line in the wallets re-export. CLAUDE.md holds the package
 rules: the multi-chain code stays in `@ecency/wallets`. Its external chain list is stale,
 since the chains under `assets/external/` are BTC, ETH, BNB and SOL.
 
-## 1. Add the key
+## 1. Failing spec first
+
+Write the builder's spec before the builder: the `queryKey` it must produce, what `queryFn` returns for a mocked response (empty and error shapes too) plus the `enabled` guard for missing arguments. Run it and confirm it fails **for the right reason**: an assertion about the missing behaviour, not an import error, a missing mock or a typo. When the subject file does not exist yet, add a stub export (a component returning `null`, a function returning `undefined`) so the run reaches the assertion. If the issue says `No test-first: <reason>` (pure layout or CSS, a dependency upgrade, infra config), skip this step and repeat that line in the PR.
+
+In either package, colocate `get-<entity>-query-options.spec.ts` beside the builder (the SDK
+picks up vitest's default spec glob, `packages/wallets/vitest.config.ts` includes
+`src/**/*.spec.ts`), then run the one spec (plus `pnpm typecheck` once it is green):
+
+```bash
+pnpm --filter @ecency/sdk test src/modules/<domain>/queries/get-<entity>-query-options.spec.ts
+pnpm --filter @ecency/wallets test src/modules/wallets/queries/<name>.spec.ts
+pnpm --filter @ecency/wallets test src/modules/assets/external/<chain>/<name>.spec.ts   # BTC, ETH, BNB, SOL
+```
+
+**Never add `--` before the path.** pnpm forwards it to vitest as a passthrough separator
+rather than a file filter, so the whole package suite runs instead of the one spec.
+
+A web query does not colocate. `apps/web/vitest.config.mts` includes
+`src/specs/**/*.spec.{ts,tsx}` only, so the spec goes under `apps/web/src/specs/api/queries/`
+and runs from the workspace root as `pnpm test src/specs/api/queries/<name>.spec.ts`.
+
+## 2. Add the key
 
 Key style is per workspace, so pick the right one before writing anything:
 
 | Workspace | Where the key comes from |
 |---|---|
 | `@ecency/sdk` | the shared `QueryKeys` object, below |
 | `@ecency/wallets` | a package-local namespaced array; `QueryKeys` is not imported there |
-| `apps/web` | the `QueryIdentifiers` enum, or a local suffix constant appended to another builder's key (step 3) |
+| `apps/web` | the `QueryIdentifiers` enum, or a local suffix constant appended to another builder's key (step 4, where the enum member is added) |
 
 `@ecency/wallets` keeps its keys local so the multi-chain code does not reach
 back into the SDK, so CLAUDE.md's blanket rule to take every cache key from `QueryKeys` is
@@ -71,15 +92,15 @@ prefix-safe. Some keys stay prefix-safe without the helper by branching on the m
 argument and returning the shorter array: `search.similarEntries`,
 `wallet.aggregatedHistory` plus `polls.vote`.
 
-## 2. Write the builder
+## 3. Write the builder
 
 File: `packages/sdk/src/modules/<domain>/queries/get-<entity>-query-options.ts`. In
 `@ecency/wallets` there are two homes: a wallet-level builder is
 `packages/wallets/src/modules/wallets/queries/<name>.ts`, while a per-chain builder for BTC,
 ETH, BNB or SOL sits directly in `packages/wallets/src/modules/assets/external/<chain>/`,
 with no `queries/` level. A web-only
 query is `apps/web/src/api/queries/<name>-query.ts` (`get-contributors-query.ts`,
-`get-gifs-query.ts`), usually with its key from `QueryIdentifiers` (step 3). The builder
+`get-gifs-query.ts`), usually with its key from `QueryIdentifiers` (step 4, where the enum member is added). The builder
 shape below carries across those homes; the imports do not. Export a function, never a
 hook, so one options object serves a component, a server prefetch or a non-React caller.
 Almost all top-level `export function` names under
@@ -178,7 +199,7 @@ type PageParam = {
     },
 ```
 
-## 3. Export it
+## 4. Export it
 
 Add one line to the domain's `queries/index.ts`, which `modules/<domain>/index.ts` already
 re-exports. A brand new domain also needs `export * from "./modules/<domain>";` in
@@ -201,7 +222,7 @@ directory can import the BTC, ETH, BNB or SOL builders. Nothing does. Adding a b
 there means adding the missing `export * from "./external";` line as well, otherwise the new
 export is unreachable.
 
-## 4. Consume it
+## 5. Consume it
 
 ```typescript
 const { data } = useQuery(getPostQueryOptions(author, permlink));
@@ -214,24 +235,6 @@ const entry = await prefetchQuery(getPostQueryOptions(author, permlink));
 Those helpers (`apps/web/src/core/react-query/query-helpers.ts`) wrap the fetch in an SSR
 timeout, so a hanging node cannot hold a render open.
 
-## 5. Test
-
-In either package, colocate `get-<entity>-query-options.spec.ts` beside the builder (the SDK
-picks up vitest's default spec glob, `packages/wallets/vitest.config.ts` includes
-`src/**/*.spec.ts`), then run the one spec plus `pnpm typecheck`:
-
-```bash
-pnpm --filter @ecency/sdk test src/modules/<domain>/queries/get-<entity>-query-options.spec.ts
-pnpm --filter @ecency/wallets test src/modules/wallets/queries/<name>.spec.ts
-```
-
-**Never add `--` before the path.** pnpm forwards it to vitest as a passthrough separator
-rather than a file filter, so the whole package suite runs instead of the one spec.
-
-A web query does not colocate. `apps/web/vitest.config.mts` includes
-`src/specs/**/*.spec.{ts,tsx}` only, so the spec goes under `apps/web/src/specs/api/queries/`
-and runs from the workspace root as `pnpm test src/specs/api/queries/<name>.spec.ts`.
-
 ## Gotchas
 
 - `enabled` gates automatic fetching only. The helpers in

.claude/skills/add-sdk-mutation/SKILL.md

@@ -8,6 +8,10 @@ argument-hint: [operation-name]
 
 Layering is in `.claude/rules/web-app.md` under "Mutation Architecture". Copy the real files below.
 
+## 0. Failing spec first
+
+Before the hook, write `use-<operation>.spec.ts` beside where it will live, for the logic the hook adds: the operation payload it builds, any cache update and the keys it invalidates. Copy the layout of `posts/mutations/use-vote.spec.ts`, which tests the vote's pure helpers rather than the React hook. Run it and confirm it fails **for the right reason**: an assertion about the missing behaviour, not an import error, a missing mock or a typo. When the subject file does not exist yet, add a stub export (a component returning `null`, a function returning `undefined`) so the run reaches the assertion. A hook that only forwards to `useBroadcastMutation` with no logic of its own needs no spec: that is the exemption below, with the reason `forwarding-only hook`. If the issue says `No test-first: <reason>` (pure layout or CSS, a dependency upgrade, infra config), skip this step and repeat that line in the PR.
+
 ## 1. SDK mutation hook
 
 `packages/sdk/src/modules/<domain>/mutations/use-<operation>.ts`. Operations come
@@ -68,9 +72,8 @@ Imports as the SDK uses them: `useBroadcastMutation` from `@/modules/core` or
 `AuthContextV2` as `import type`, usually from `@/modules/core/types`.
 `invalidateAfterBroadcast` already defers on async broadcasts, so no hand rolled
 `setTimeout`. Other models:
-`posts/mutations/use-vote.ts`, `use-reblog.ts`, `use-comment.ts`. Co-locate a
-`use-<operation>.spec.ts` when the hook has logic of its own; most hooks have none,
-so copy the pattern from `posts/mutations/use-vote.spec.ts`.
+`posts/mutations/use-vote.ts`, `use-reblog.ts`, `use-comment.ts`. The spec from step 0, when there is
+one, goes green here.
 
 Export with `export * from "./use-<operation>";` in the domain's
 `mutations/index.ts`. The chain up to `packages/sdk/src/index.ts` is usually
@@ -116,7 +119,7 @@ named export to `apps/web/src/api/sdk-mutations/index.ts`.
 
 ## 4. Verify
 
-The web app resolves `@ecency/sdk` through `dist`,
+The step 0 spec passes and the PR body lists each spec path, the command and the assertion that failed before the change, then the green run after it; with no spec, the PR carries the `No test-first: <reason>` line. The web app resolves `@ecency/sdk` through `dist`,
 so `pnpm typecheck` reports your new export as missing from the package until the SDK
 is rebuilt:
 

.claude/skills/add-test/SKILL.md

@@ -15,6 +15,8 @@ Procedure for `apps/web` (`@ecency/web`) specs. Background you should not re-rea
   `AccountBuilder`). It does not mention `mockFeatureFlags` or `waitForMs`, so read
   `test-utils.tsx` itself before assuming a helper is missing.
 
+When a spec covers behaviour being added or fixed, write it before the code, from the issue's **Acceptance tests** list. Run it and confirm it fails **for the right reason**: an assertion about the missing behaviour, not an import error, a missing mock or a typo. When the subject file does not exist yet, add a stub export (a component returning `null`, a function returning `undefined`) so the run reaches the assertion. A spec that pins behaviour which already works (a test-only PR, a refactor) has no red run; say so in the PR.
+
 Sections 1-4 cover what usually breaks.
 
 ## 1. Where the file goes
@@ -175,4 +177,5 @@ vi.mocked(useQuery).mockImplementation(() => {
 - [ ] `import "@testing-library/jest-dom"` when asserting on the DOM
 - [ ] `@/utils` and `@ecency/sdk` re-mocked with `importActual` for anything past the global stubs
 - [ ] Assertions target i18n keys, not English strings
+- [ ] For new or fixed behaviour: spec was run red before the change, failing on its assertion
 - [ ] `pnpm test <path>` passes with no `--`, then `pnpm test` for the full suite

.claude/skills/code-review/SKILL.md

@@ -85,6 +85,19 @@ modals use CSS transitions (`apps/web/src/features/ui/modal/index.tsx`). Do not
 reintroduce it.
 
 **Tests**
+- **A spec for added or fixed behaviour must fail without the change.** Check it: revert
+  only the source part of the diff (keep the spec), run the spec and confirm it goes red on
+  the assertion it names, then restore. A source file the PR adds cannot be reverted that
+  way, since the spec would then fail on its import: replace it with a stub export (a
+  component returning `null`, a function returning `undefined`) instead. For a new guard,
+  delete just the guard. A spec that stays green only exercises the path where the change
+  does nothing: flag it as `BUG`. Restore from a copy of the file, never
+  `git checkout -- <file>`, which drops uncommitted work.
+- Specs that pin behaviour which already works (a test-only PR, a refactor) are expected
+  to stay green and are not a finding.
+- The PR body lists each spec path, the command, the assertion that failed before the
+  change and the green run after it. A change with no spec repeats the issue's
+  `No test-first: <reason>`.
 - Placement decides collection. Some workspaces pin `include`, so a misplaced or
   wrongly suffixed file there is silently never run: `apps/web` takes
   `src/specs/**/*.spec.{ts,tsx}` only, `packages/wallets` `src/**/*.spec.ts`,

.claude/skills/debug/SKILL.md

@@ -23,6 +23,10 @@ invisible. Most invariants below have a spec; find it before a repro; web specs
 one browser only, or only right after a deploy, is often already handled by the
 scripts in `apps/web/public/scripts/`; check those first.
 
+## First: a failing spec that reproduces the bug
+
+Before changing any code, write a spec that fails the way the report does, driven through the real code path with the reporter's real input (not a simplified harness that can drop the cause). Use the triage below to find where. Run it and confirm it fails **for the right reason**: an assertion about the missing behaviour, not an import error, a missing mock or a typo. When the subject file does not exist yet, add a stub export (a component returning `null`, a function returning `undefined`) so the run reaches the assertion. If it passes on the unchanged code, that is not a fix: either the report does not reproduce on current code or the spec misses the reporter's input. Stop, recheck the input, steps and assertion, then report that before writing any fix. The fix is done when that spec goes from red to green and the rest of the suite still passes; keep the spec. A bug that only shows in a real browser or on a live node (layout, CDN, network) may not fit a spec; then record the manual repro steps in the issue and say why there is no spec.
+
 ## Post shown as deleted, 404, or spinning
 
 `getPostQueryOptions(author, permlink?, observer = "", num?)`

.github/ISSUE_TEMPLATE/spec.yml

@@ -0,0 +1,33 @@
+name: Spec (feature or bug)
+description: Work item with acceptance tests written before the code, or a stated reason why there are none
+body:
+  - type: textarea
+    id: problem
+    attributes:
+      label: Problem
+      description: What is wrong or missing and who sees it. For a bug, give the exact steps, the input that triggers it, what happens and what should happen.
+    validations:
+      required: true
+  - type: textarea
+    id: proposal
+    attributes:
+      label: Proposal
+      description: The change, at the level of behaviour. Name files only when the location is already known.
+    validations:
+      required: true
+  - type: textarea
+    id: acceptance-tests
+    attributes:
+      label: Acceptance tests
+      description: |
+        Each line becomes a spec that is written BEFORE the code, run and seen failing on its assertion, then made green by the change. For a bug, the first line reproduces the report with the real input.
+        If a spec written first adds nothing (pure layout or CSS, a dependency upgrade, infra config), write one line instead: `No test-first: <reason>`
+      placeholder: |
+        - [ ] Given <state>, when <action>, then <result>. Spec: `<path to spec file>`
+    validations:
+      required: true
+  - type: textarea
+    id: out-of-scope
+    attributes:
+      label: Out of scope
+      description: Optional. Related things this issue deliberately leaves alone.

CLAUDE.md

@@ -104,7 +104,7 @@ node scripts/origin-config-audit.mjs --fail
 pnpm test src/specs/path/to/test.spec.tsx
 
 # For packages (Vitest)
-pnpm --filter @ecency/sdk test -- path/to/test.spec.ts
+pnpm --filter @ecency/sdk test path/to/test.spec.ts   # no `--`: with it, the whole package suite runs
 ```
 
 ## Monorepo Architecture
@@ -241,7 +241,9 @@ The web app will not see package changes until they are rebuilt.
 ## Pull Requests
 
 - Branch from `develop` (not `master`)
-- Include tests with changes
+- Test first: issues carry a required **Acceptance tests** field (`.github/ISSUE_TEMPLATE/spec.yml`); write those
+  specs before the code, see them fail for the right reason, then make them green. The PR body lists each spec path, the command and the assertion that failed before the change, then the green run after it,
+  or repeats the issue's `No test-first: <reason>`
 - Update tests if modifying existing code
 - Mark PR as WIP if not ready
 - Include description, screenshots/videos, and link to issues