docs(workflow): add a Verification plan to critical-design-review #166

Merged

marco merged 1 commit from docs/design-review-verification-plan into main

2026-07-23 19:22:48 +02:00

marco commented

2026-07-23 19:05:26 +02:00

Owner

Summary

Workflow change: the critical-design-review skill now emits a Verification plan as part of its output — a preview, at design time, of how a feature will be proven.

New output-contract item (7. Verification plan, between Recommendation and Open decisions): bullets for the test levels that apply (unit vs World, WebApplicationFactory + SQLite :memory:, WebSocket flow), the trust-boundary case that must be covered (e.g. non-admin rejected, out-of-range/forged intent dropped, persistence round-trip), and — only for a visible change — the debug-harness screenshot path (fresh DB → drive to state → which states), else N/A — no visible surface.
Framed to avoid the two failure modes we discussed: it's a testability gate (a design you can't credibly verify, especially its server-side validation, lowers the Verdict — not paperwork), and a preview not a frozen script (the PR's How it was tested stays the source of truth). Explicit delta vs the DoD: DoD = the what (objectively verifiable acceptance criteria), Verification plan = the how.
Persisted into the Forgejo design issue alongside the Invariants Check (item 9).
One-line pointer added to CLAUDE.md's "Critical review before implementing a feature" rule.

How it was tested

Docs/skill only — no compiled code changed. dotnet build/tests are unaffected; CI's lint+test gate still runs and passes. The change itself is exercised on the next feature design that goes through the skill.

Checklist

Markdown/skill only; whole solution still builds + tests green
Skill remains the single source of truth for the review's sections; CLAUDE.md just points to it
No self-merge — owner merges

## Summary Workflow change: the `critical-design-review` skill now emits a **Verification plan** as part of its output — a preview, at design time, of *how* a feature will be proven. - New output-contract item (**7. Verification plan**, between Recommendation and Open decisions): bullets for the **test levels** that apply (unit vs `World`, `WebApplicationFactory` + SQLite `:memory:`, WebSocket flow), the **trust-boundary case that must be covered** (e.g. non-admin rejected, out-of-range/forged intent dropped, persistence round-trip), and — **only for a visible change** — the debug-harness **screenshot path** (fresh DB → drive to state → which states), else `N/A — no visible surface`. - Framed to avoid the two failure modes we discussed: it's a **testability gate** (a design you can't credibly verify, especially its server-side validation, lowers the Verdict — not paperwork), and a **preview not a frozen script** (the PR's *How it was tested* stays the source of truth). Explicit **delta vs the DoD**: DoD = the *what* (objectively verifiable acceptance criteria), Verification plan = the *how*. - Persisted into the Forgejo design issue alongside the Invariants Check (item 9). - One-line pointer added to `CLAUDE.md`'s "Critical review before implementing a feature" rule. ## How it was tested Docs/skill only — no compiled code changed. `dotnet build`/tests are unaffected; CI's lint+test gate still runs and passes. The change itself is exercised on the next feature design that goes through the skill. ## Checklist - [x] Markdown/skill only; whole solution still builds + tests green - [x] Skill remains the single source of truth for the review's sections; CLAUDE.md just points to it - [x] No self-merge — owner merges

marco added 3 commits

2026-07-23 19:05:26 +02:00

fix: post-review hardening — invariant docs, illegal-move metric, JWT/save robustness + tests

ci / Lint & Test (pull_request) Failing after 1m14s

Details

81f6830bc6

- docs: correct 5 component comments that still described the pre-#54 'single lock'
  model (now single-threaded, no lock) — a load-bearing invariant.
- metrics: count illegal moves (into wall/off-map/onto a creature) in
  intents_rejected_total; cooldown/overweight pacing stays uncounted.
- security/integrity: overflow-safe Chebyshev distance (hostile client coords),
  pin HS256 on Auth's JWT bearer, reject missing password with 400, fail loud on a
  corrupt-body world save (don't silently overwrite), log dropped unknown creature types.
- tests: negative JWT (wrong key/issuer/audience) + admin-authorization e2e at /ws,
  illegal-move count, overflow distance, corrupt-save, missing-password.

build: pin analyzer versions (Roslynator 4.15.0, SonarAnalyzer 10.29.0.143774)

ci / Lint & Test (pull_request) Successful in 1m40s

Details

7ba2bb00f0

Both were floating (4.*/10.*), so CI's fresh restore pulled a newer SonarAnalyzer
that tightened S8969 and failed the build while local passed — a non-reproducible
build. Pin exactly like every other package; bump analyzers deliberately in future.

docs(workflow): add a Verification plan to critical-design-review output

ci / Lint & Test (pull_request) Has been cancelled

Details

11be8b24c7

Every feature design now previews HOW it'll be proven — test levels (unit/World,
WebApplicationFactory, WebSocket flow), the trust-boundary case that must be
covered, and (only if visible) the debug-harness screenshot path. It's a
testability gate: a design you can't credibly verify lowers the verdict. Distinct
from the DoD (the what vs the how); persisted into the design issue; a preview the
PR reconciles, not a frozen script.

marco force-pushed docs/design-review-verification-plan from 11be8b24c7

ci / Lint & Test (pull_request) Has been cancelled

Details

to ba703fcae0

ci / Lint & Test (pull_request) Successful in 4m53s

Details

2026-07-23 19:07:48 +02:00

Compare

marco force-pushed docs/design-review-verification-plan from ba703fcae0

ci / Lint & Test (pull_request) Successful in 4m53s

Details

to 2e58e5173f