From 982a437523e91bca79955720ef8dd6ccef4c87e9 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 13 Jun 2026 11:56:25 +0200 Subject: [PATCH 01/10] docs(adr): adopt Spec-Driven Development (ADR-041) Records the decision to layer SDD (EARS requirements, OpenSpec-style delta artifacts, a versioned constitution, and AGENTS.md) on top of the existing Gitea-issue + multi-persona-review workflow. Numbered 041 to extend the existing docs/adr/ archive rather than starting a parallel one. Co-Authored-By: Claude Opus 4.8 --- docs/adr/041-sdd-adoption.md | 87 ++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 docs/adr/041-sdd-adoption.md diff --git a/docs/adr/041-sdd-adoption.md b/docs/adr/041-sdd-adoption.md new file mode 100644 index 00000000..e414a418 --- /dev/null +++ b/docs/adr/041-sdd-adoption.md @@ -0,0 +1,87 @@ +# ADR-041 — Adopt Spec-Driven Development (SDD) + +**Status:** Accepted +**Date:** 2026-06-13 +**Issue:** SDD integration (docs/sdd-integration branch) + +> This is the "ADR-000" the SDD scaffold refers to, numbered 041 to fit the existing archive +> sequence rather than starting a parallel one. See [`.specify/adrs/README.md`](../../.specify/adrs/README.md). + +## Context + +The project already runs a lightweight, multi-persona review loop: feature work is tracked +as dense Gitea issues, and the `review-issue` / `review-pr` / `deliver-issue` skills dispatch +the character personas in `.claude/personas/` (requirements engineer, developer, security, +devops, ui, architect, tester) to comment on issues and PRs. `COLLABORATING.md` already +mandates a Research → Plan → Implement → Validate cycle, red/green TDD, and a User-Journey + +E2E-Scenario section on every feature issue. Decisions are captured in this ADR archive. + +What is missing is a **machine-readable, uniform front-end to that workflow**: + +- Requirements are written in free prose, so two readers (and an AI agent) can interpret the + same issue differently. There is no enforced requirement grammar and no stable requirement + identifier to trace from spec → code → test. +- There is no single short file an AI coding agent reads on every invocation; the rules are + spread across `CLAUDE.md`, `COLLABORATING.md`, `CODESTYLE.md`, and `CONTRIBUTING.md`. +- Persona review is valuable but ad-hoc — there is no per-role checklist that gates a spec + *before* implementation, so blind spots surface during PR review instead of at spec time. +- There is no living traceability record and no CI signal that a spec is well-formed. + +The project is solo + LLM-driven; spec density and machine-readability are exactly the +leverage points that make agent output reliable. + +## Decision + +Adopt Spec-Driven Development, layered **on top of** the existing workflow, not replacing it: + +1. **EARS requirements.** Every feature requirement is written in one of the five EARS + patterns and carries a per-feature `REQ-NNN` id. (constitution §3; templates/feature-spec.md) +2. **OpenSpec-style delta artifacts.** Each in-flight feature gets a `.specify/features//` + directory holding `spec.md`, `design.md`, optional `api-contract.yaml` / `threat-model.md` + / feature-local ADRs, `tasks.md`, and `checklist-results.md`. Artifacts are deltas focused + on one change, archived when the feature ships. +3. **A constitution + AGENTS.md.** `.specify/constitution.md` records the few non-negotiable + rules (semantically versioned); `.specify/AGENTS.md` is the short machine-readable file AI + agents read every invocation, cross-referencing the constitution rather than duplicating it. +4. **Persona checklists.** `.specify/personas/*.md` turn the existing rich personas into + concise, EARS-aware, pass/fail spec-review checklists that gate a spec before code. +5. **Living RTM + CI gate.** `.specify/rtm.md` traces every `REQ-NNN`; `.gitea/workflows/sdd-gate.yml` + lints spec structure, validates contracts, and checks traceability (non-blocking initially). +6. **Reuse, don't duplicate.** ADRs stay in `docs/adr/`; persona checklists reference + `.claude/personas/`; the Gitea issue templates mirror the feature-spec template. + +## Alternatives Considered + +| Option | Pros | Cons | Reason rejected | +|---|---|---|---| +| Adopt SDD (EARS + delta artifacts + AGENTS.md), integrated with existing docs | Adds requirement rigor & machine-readability; reuses ADR archive and personas; incremental | Per-feature spec authoring overhead | **Chosen** | +| No change (keep free-prose issues + persona review) | Zero new process | Ambiguous requirements; no traceability; no single agent-facing file; blind spots found late | Leaves the exact gaps that hurt agent output | +| Full GitHub Spec Kit | Mature, opinionated tooling | Heavy, opinionated structure that fights the existing Gitea/skill/persona workflow; redundant ADR/persona machinery | Too much to retrofit; conflicts with what already works | +| BMAD-METHOD | Rich agent-role framework | Large conceptual surface for a solo project; overlaps the existing persona system | Over-engineered for this team size | + +## Consequences + +- **Cost:** ~30–60 min of spec authoring + structured persona review per feature, before + implementation begins. +- **Gains:** unambiguous, testable requirements; a stable spec→code→test trace; one + authoritative agent-facing rules file; review blind spots caught at spec time; better and + more consistent AI-agent output (the project's primary delivery mechanism). +- **Obligations created:** + - The constitution is semantically versioned; any change triggers its Sync Impact review. + - A new feature follows the workflow in [SPEC_DRIVEN_DEVELOPMENT.md](../../SPEC_DRIVEN_DEVELOPMENT.md). + - `rtm.md` is kept in sync as requirements land (CI warns on drift). + - The SDD CI jobs start non-blocking and flip to blocking once adoption settles (TODO noted + in the workflow). +- **Non-disruptive:** the existing Gitea-issue, branch/PR, TDD, and persona-review practices + are unchanged in spirit — SDD formalises their inputs, it does not replace them. + +## References + +- [.specify/constitution.md](../../.specify/constitution.md), [.specify/AGENTS.md](../../.specify/AGENTS.md) +- [SPEC_DRIVEN_DEVELOPMENT.md](../../SPEC_DRIVEN_DEVELOPMENT.md) +- [COLLABORATING.md](../../COLLABORATING.md), [CONTRIBUTING.md](../../CONTRIBUTING.md) +- EARS: Mavin et al., "Easy Approach to Requirements Syntax" (2009) + +## Revision log + +- 2026-06-13 — constitution v1.0.0 ratified with this ADR. -- 2.49.1 From 01f51854f6eb007a07af2602064746ddc547f01b Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 13 Jun 2026 11:56:31 +0200 Subject: [PATCH 02/10] =?UTF-8?q?feat(sdd):=20add=20.specify=20scaffold=20?= =?UTF-8?q?=E2=80=94=20constitution,=20AGENTS,=20personas,=20templates,=20?= =?UTF-8?q?example,=20RTM?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces the SDD root: a v1.0.0 constitution and machine-readable AGENTS.md grounded in the project's real conventions; six EARS-aware persona spec-review checklists that cross-reference .claude/personas/; feature-spec/ADR/threat-model/ api-contract templates; a fully worked _example feature; a living RTM; and an adrs/ pointer that reuses the existing docs/adr/ archive. Co-Authored-By: Claude Opus 4.8 --- .specify/AGENTS.md | 76 ++++++++++ .specify/adrs/README.md | 25 ++++ .specify/constitution.md | 80 +++++++++++ .../adr-001-avatars-reuse-archive-bucket.md | 46 ++++++ .specify/features/_example/api-contract.yaml | 136 ++++++++++++++++++ .../features/_example/checklist-results.md | 76 ++++++++++ .specify/features/_example/design.md | 63 ++++++++ .specify/features/_example/spec.md | 118 +++++++++++++++ .specify/features/_example/tasks.md | 47 ++++++ .specify/features/_example/threat-model.md | 45 ++++++ .specify/personas/architect.md | 40 ++++++ .specify/personas/developer.md | 39 +++++ .specify/personas/devops.md | 39 +++++ .specify/personas/requirements-engineer.md | 43 ++++++ .specify/personas/security.md | 42 ++++++ .specify/personas/ui-ux.md | 39 +++++ .specify/rtm.md | 36 +++++ .specify/templates/adr.md | 42 ++++++ .specify/templates/api-contract-stub.md | 94 ++++++++++++ .specify/templates/feature-spec.md | 89 ++++++++++++ .specify/templates/threat-model.md | 51 +++++++ 21 files changed, 1266 insertions(+) create mode 100644 .specify/AGENTS.md create mode 100644 .specify/adrs/README.md create mode 100644 .specify/constitution.md create mode 100644 .specify/features/_example/adr-001-avatars-reuse-archive-bucket.md create mode 100644 .specify/features/_example/api-contract.yaml create mode 100644 .specify/features/_example/checklist-results.md create mode 100644 .specify/features/_example/design.md create mode 100644 .specify/features/_example/spec.md create mode 100644 .specify/features/_example/tasks.md create mode 100644 .specify/features/_example/threat-model.md create mode 100644 .specify/personas/architect.md create mode 100644 .specify/personas/developer.md create mode 100644 .specify/personas/devops.md create mode 100644 .specify/personas/requirements-engineer.md create mode 100644 .specify/personas/security.md create mode 100644 .specify/personas/ui-ux.md create mode 100644 .specify/rtm.md create mode 100644 .specify/templates/adr.md create mode 100644 .specify/templates/api-contract-stub.md create mode 100644 .specify/templates/feature-spec.md create mode 100644 .specify/templates/threat-model.md diff --git a/.specify/AGENTS.md b/.specify/AGENTS.md new file mode 100644 index 00000000..0f52319a --- /dev/null +++ b/.specify/AGENTS.md @@ -0,0 +1,76 @@ +# AGENTS.md + +Machine-readable rules for AI coding agents (Claude Code, Copilot, Cursor, …) working in +this repository. Read this on every invocation. These are **executable constraints**, not +aspirations. The full rationale lives in [constitution.md](./constitution.md) and the docs +it links — this file does not duplicate it, it points to it. + +If anything here conflicts with the user's explicit instruction, the user wins. Otherwise, +constitution > this file > convenience. + +--- + +## Stack & Versions + +| Layer | Tech | Version | +|---|---|---| +| Backend | Spring Boot (Java, Maven, Jetty, JPA/Hibernate, Flyway, Spring Security, Session JDBC) | Boot 4.0.6 / Java 21 | +| API docs | springdoc-openapi (webmvc-ui), served at `/v3/api-docs` (dev profile only) | — | +| Frontend | SvelteKit / Svelte | 2.60 / 5.43 | +| Frontend lang/style | TypeScript / Tailwind CSS / Paraglide i18n (de/en/es) | TS 5.9 / TW 4.1 | +| API client | `openapi-fetch` + `openapi-typescript` (types generated from the live spec) | — | +| DB | PostgreSQL | 16 | +| Object storage | MinIO (S3-compatible) | — | +| Sidecars | `ocr-service`, `nlp-service` (Python / FastAPI) | Python 3.11 | +| Tests | JUnit + Mockito + `@WebMvcTest` + Testcontainers (backend); Vitest + `vitest-browser-svelte` + Playwright (frontend); Pytest (services) | — | +| Lint/format | ESLint 9 (+ `eslint-plugin-boundaries`) + Prettier; Semgrep (backend) | — | +| CI | Gitea Actions (`.gitea/workflows/`) | — | + +App port `8080`; management port `8081`. Backend app id: `org.raddatz.familienarchiv` / `0.0.1-SNAPSHOT`. + +## Architectural Constraints + +- Controllers call services only — never a repository. (constitution §1.2) +- A service uses only its own domain's repository; reach other domains via their service. (constitution §1.3) +- A new backend domain goes in its own package AND is added to `ArchitectureTest`'s allow-lists in the same change. (constitution §1.7) +- Frontend cross-domain imports are allowed only where `frontend/eslint.config.js` permits; otherwise move shared code to `$lib/shared/`. (constitution §1.4) +- Never serialize a lazy-collection entity across the controller boundary — assemble a view in-transaction. (constitution §1.6 / ADR-036) +- `Person` ≠ `AppUser`; do not add account guards to Person-domain operations. (constitution §1.5) +- Every `POST/PUT/PATCH/DELETE` endpoint has `@RequirePermission(Permission.X)`. Use the enum, never `@PreAuthorize`. (constitution §2.1–2.2) +- Throw only `DomainException.notFound/forbidden/conflict/internal()` from services, each with an `ErrorCode`. (CONTRIBUTING §Error handling) +- Set `createdBy`/`updatedBy` from the session principal in the service — never bind them from a request body. (constitution §2.4) +- Add an `@Schema(requiredMode = REQUIRED)` to every always-populated field. (constitution §3.5) +- Never introduce a new runtime dependency without an ADR in `Accepted` status. (constitution §5.1) +- Render untrusted text with `{...}`; never `{@html}` on user/import data. (constitution §2.5) +- Build dates from ISO strings with a `T12:00:00` suffix. (constitution §3.7) + +## Workflow Rules + +- Always write a failing test before implementation code; confirm it fails, then make it pass, then refactor. (constitution §3.1) +- Run only the specific test file/class locally — never the full suite (it crashes the machine); leave the full sweep to CI. +- Run `npm run generate:api` (in `frontend/`) after ANY backend model or endpoint change — most common cause of TS errors. +- Run `npm run lint` before every commit; a fresh frontend worktree needs `npm install` first or the pre-commit hook fails. +- When adding a new `ErrorCode`, update all four sites at once (constitution §3.6). +- One logical change per commit; reference the Gitea issue (`Closes #n` / `Refs #n`) on the last line. +- Create a git worktree for new issue work — never `git checkout -b` in the main repo while another branch has in-flight work. Avoid `+` in worktree/branch names (breaks vitest browser mode). +- Pull `main` as a separate explicit step before creating a branch. +- Track work as Gitea issues (`http://192.168.178.71:3005`, repo `marcel/familienarchiv`), not todo files. +- Verify ADR and Flyway migration numbers against disk before using one — parallel worktrees make issue-body numbers go stale. + +## Do Not Touch + +- Generated: `frontend/src/lib/generated/api.ts`, `frontend/src/lib/paraglide/`, `frontend/.svelte-kit/`, `frontend/build/`, `backend/target/`. +- Shipped Flyway migrations — add a new forward-only migration instead. +- An `Accepted` ADR — supersede it with a new one. +- `actions/(upload|download)-artifact` version — stays at `@v3` (ADR-014). +- CI guard steps — do not remove/weaken without an ADR. +- `main` — never commit directly; branch + PR only. +- Worktree copies (`familienarchiv-*`, `.worktrees/`) and `data/` — never commit. + +## Spec-Driven Development + +Before implementing a feature, read its spec at `.specify/features//spec.md` and its +contract at `.specify/features//api-contract.yaml` if present. The spec's EARS +requirements (`REQ-NNN`) are the contract; each maps to a test. Worked reference: +[`.specify/features/_example/`](./features/_example/). Full workflow: +[SPEC_DRIVEN_DEVELOPMENT.md](../SPEC_DRIVEN_DEVELOPMENT.md). diff --git a/.specify/adrs/README.md b/.specify/adrs/README.md new file mode 100644 index 00000000..190052f7 --- /dev/null +++ b/.specify/adrs/README.md @@ -0,0 +1,25 @@ +# ADR archive — see `docs/adr/` + +This project already keeps a mature, permanent ADR archive at +[`../../docs/adr/`](../../docs/adr/) (40+ records, format `NNN-kebab-title.md`). SDD does +**not** introduce a second archive — that would split the project's decision history in two. + +## Where ADRs live + +- **Project-wide decisions** → [`docs/adr/NNN-kebab-title.md`](../../docs/adr/). Use the + next free `NNN` (verify against the directory on disk — parallel worktrees make + issue-body numbers stale). Template: [`../templates/adr.md`](../templates/adr.md). +- **The decision to adopt SDD itself** → + [`docs/adr/041-sdd-adoption.md`](../../docs/adr/041-sdd-adoption.md) (this is the + "ADR-000" the SDD scaffold calls for, numbered to fit the existing sequence). +- **Feature-local decisions** that are only meaningful within one in-flight feature → + beside that feature's spec, e.g. + [`../features/_example/adr-001-avatars-reuse-archive-bucket.md`](../features/_example/adr-001-avatars-reuse-archive-bucket.md). + Promote one to `docs/adr/` if its reach turns out to be project-wide. + +## Rules (unchanged from the existing convention) + +- An ADR is **immutable once `Accepted`** — supersede it with a new, higher-numbered ADR; + set the old one's status to `Superseded by ADR-MMM`. +- Header style matches the existing archive: `# ADR-NNN — Title`, then + `**Status:** / **Date:** / **Issue:**`. diff --git a/.specify/constitution.md b/.specify/constitution.md new file mode 100644 index 00000000..e1dfcdb9 --- /dev/null +++ b/.specify/constitution.md @@ -0,0 +1,80 @@ +# Familienarchiv Constitution + +**Version:** v1.0.0 +**Status:** Ratified +**Date:** 2026-06-13 +**Adoption ADR:** [docs/adr/041-sdd-adoption.md](../docs/adr/041-sdd-adoption.md) + +> The non-negotiable rules of this project. Every spec, every PR, and every AI agent is +> bound by this document. Rules here are deliberately few and absolute — guidance and +> rationale live in [CLAUDE.md](../CLAUDE.md), [COLLABORATING.md](../COLLABORATING.md), +> [CODESTYLE.md](../CODESTYLE.md), [CONTRIBUTING.md](../CONTRIBUTING.md), and the ADR +> archive ([docs/adr/](../docs/adr/)). When this file conflicts with any of those, **this +> file wins** — open an ADR to change it. +> +> Versioning is semantic: **MAJOR** = a rule removed or weakened (existing code may now +> violate the constitution), **MINOR** = a rule added or tightened, **PATCH** = wording +> only. Any change requires the Sync Impact review in the last section. + +--- + +## 1. Architecture Principles + +1. The backend is organised package-by-domain under `org.raddatz.familienarchiv`; a new domain lives in its own package, never spread across layer packages. +2. Controllers never call repositories directly — a controller calls only services. +3. A service accesses only its own domain's repository; cross-domain data is fetched through the other domain's service, never its repository. +4. The frontend mirrors the backend domain split under `frontend/src/lib//`, and cross-domain imports are allowed only where `frontend/eslint.config.js` (`boundaries/dependencies`) permits them. +5. A `Person` (historical subject) and an `AppUser` (login account) are distinct domains and never share an identity or an account guard. +6. Lazy-collection-bearing entities are never serialized across the controller boundary; the owning service assembles an explicit view inside the transaction (see [ADR-036](../docs/adr/036-geschichte-responses-are-views-not-entities.md)). +7. A new backend domain package is added to `ArchitectureTest`'s package allow-lists in the same change that introduces it. +8. Synchronous cross-domain side effects use in-transaction domain events, not direct service-to-service write calls (see [ADR-006](../docs/adr/006-synchronous-domain-events-in-transaction.md)). + +## 2. Security Defaults + +1. Every `POST`, `PUT`, `PATCH`, and `DELETE` endpoint carries `@RequirePermission(Permission.X)` — there is no unguarded mutating endpoint. +2. Authorization uses the typed `Permission` enum and `@RequirePermission`, never magic-string `@PreAuthorize`. +3. All user input is validated at the system boundary (controller / form action), and validation failures return a typed `ErrorCode`, never a raw exception. +4. Audit fields (`createdBy`/`updatedBy`) are set from the session principal inside the service and are never bound from a request body. +5. Untrusted text is rendered through Svelte's default `{...}` escaping; `{@html}` is never used on user- or import-derived strings. +6. Secrets are read only from environment variables (see `.env.example`); no secret, token, password, or DSN is ever committed to the repository or written to a log. +7. Logs never contain PII beyond a stable user/entity UUID — no names, email addresses, document contents, or transcription text. +8. Every state-mutating endpoint is covered by an Unwanted-behavior requirement (EARS `If`) describing the unauthenticated/unauthorized response. +9. A dependency security audit runs on every CI run (`npm audit --audit-level=high` frontend, Semgrep `.semgrep/security.yml` backend) and nightly; a `high` finding blocks merge. + +## 3. Code Quality Rules + +1. All new behavior is driven by a failing test written before the implementation (Red → Green → Refactor); a passing-on-first-run test proves nothing and is rejected. +2. KISS beats DRY — no premature abstraction; an abstraction is introduced only on the third real caller. +3. Each commit does exactly one logical thing and references its Gitea issue (`Closes #n` / `Refs #n`) on the last line of the body. +4. No backwards-compatibility shims are added for code that has no callers. +5. Every entity/DTO field the backend always populates carries `@Schema(requiredMode = REQUIRED)`, and `npm run generate:api` is run after any backend model or endpoint change. +6. A new `ErrorCode` is added in all four places at once: `ErrorCode.java`, `frontend/src/lib/shared/errors.ts`, `getErrorMessage()`, and `messages/{de,en,es}.json`. +7. Dates built from an ISO date string append `T12:00:00` to avoid UTC off-by-one. +8. `npm run lint` (Prettier + ESLint, including the domain boundary rule) passes before every commit. + +## 4. Do-Not-Touch List + +1. Do not edit generated artifacts: `frontend/src/lib/generated/api.ts`, `frontend/src/lib/paraglide/`, `frontend/.svelte-kit/`, `frontend/build/`, `backend/target/`. +2. Do not edit an `Accepted` ADR — supersede it with a new, higher-numbered ADR. +3. Do not upgrade `actions/upload-artifact` / `download-artifact` past `@v3` (Gitea act_runner lacks the v4 protocol — [ADR-014](../docs/adr/014-upload-artifact-v3-pin.md)). +4. Do not remove or weaken a CI guard step (banned-pattern greps, self-tested regexes) without an ADR recording why. +5. Do not commit to `main` directly — all work flows through a branch and a PR. +6. Do not edit a Flyway migration that has shipped; add a new forward-only migration instead. +7. Do not commit the worktree copy directories (`familienarchiv-*`, `.worktrees/`) or `data/`. + +## 5. Dependency Policy + +1. A new runtime dependency (backend `pom.xml` or frontend `dependencies`) requires an ADR in `Accepted` status before it is merged. +2. A new dependency must be version-pinned in the manifest, and any exact pin (no caret) carries a comment stating why it cannot float (see the `@vitest/browser-playwright` pin). +3. Renovate manages dependency-update PRs; a major-version bump is treated as a feature requiring its own spec and review, not an auto-merge. +4. A dependency with an unresolved `high`+ advisory is not merged; it is pinned to a safe version or replaced. + +## 6. Sync Impact + +When this constitution changes, the author MUST, in the same PR: + +1. Bump the **Version** header per the semantic rule above and record the change in [docs/adr/041-sdd-adoption.md](../docs/adr/041-sdd-adoption.md)'s revision log (or a superseding ADR for a MAJOR change). +2. Re-read and reconcile every file that restates a rule changed here: `CLAUDE.md`, `COLLABORATING.md`, `CODESTYLE.md`, `CONTRIBUTING.md`, `.specify/AGENTS.md`, and the affected `.specify/personas/*.md` checklists. +3. Update any `.specify/templates/*` section that quotes a changed rule. +4. Run the `constitution-diff` CI job locally (or read its PR comment) and resolve every file it lists. +5. Announce the version bump in the PR description so reviewers re-read the constitution before approving. diff --git a/.specify/features/_example/adr-001-avatars-reuse-archive-bucket.md b/.specify/features/_example/adr-001-avatars-reuse-archive-bucket.md new file mode 100644 index 00000000..edff1872 --- /dev/null +++ b/.specify/features/_example/adr-001-avatars-reuse-archive-bucket.md @@ -0,0 +1,46 @@ +# ADR-001 (feature-local) — Avatars reuse the archive bucket under an `avatars/` prefix + +**Status:** Accepted +**Date:** 2026-06-13 +**Issue:** # (profile picture upload) + +> **Feature-local ADR.** This decision is scoped to the avatar feature and lives with its +> spec. A decision with project-wide reach is promoted to the permanent archive at +> `docs/adr/` with the next free number. (For the worked example, it stays local.) + +## Context + +Avatars are small binary objects keyed per user. The project already runs MinIO with a +single archive bucket and a `FileService` abstraction used by document uploads. We must +decide where avatar bytes live without adding operational surface that the self-hosted +Compose deployment has to learn about. + +## Decision + +Store each avatar in the **existing archive bucket** under the deterministic key +`avatars/{userId}`, written and read through the existing `FileService`. No new bucket, no +new env var, no new Compose service or bucket-bootstrap step. + +## Alternatives Considered + +| Option | Pros | Cons | Reason rejected | +|---|---|---|---| +| Reuse archive bucket, `avatars/` prefix | No infra change; reuses `FileService`; idempotent overwrite | Mixes avatars with documents in one bucket | **Chosen** — least operational cost; prefix keeps them logically separate | +| Dedicated `avatars` bucket | Clean separation; independent lifecycle/policy | New bucket + bootstrap step + env var + Compose idempotency test | Operational overhead not justified for small, low-value objects | +| Store bytes in PostgreSQL (`bytea`) | One datastore; transactional with the row | Bloats the DB and backups; streaming images via JPA is awkward | Wrong tool; MinIO already exists for blobs | +| External CDN / object store | Offloads bandwidth | New third-party dependency + secret + ADR; conflicts with self-hosted goal | Contradicts the self-hosted infrastructure stance | + +## Consequences + +- No deployment change ships with this feature — only a Flyway column and code. +- Avatars and documents share a bucket; any future per-object lifecycle policy must filter + by the `avatars/` prefix. +- The deterministic key (`avatars/{userId}`, no random suffix) makes replace an overwrite, + so there is no orphan-cleanup obligation (REQ-001). +- If avatars later need independent retention or a public CDN, this ADR is superseded by a + project-wide ADR in `docs/adr/`. + +## References + +- [`./spec.md`](./spec.md), [`./design.md`](./design.md) +- [constitution §5 Dependency Policy](../../constitution.md#5-dependency-policy) diff --git a/.specify/features/_example/api-contract.yaml b/.specify/features/_example/api-contract.yaml new file mode 100644 index 00000000..19685e1d --- /dev/null +++ b/.specify/features/_example/api-contract.yaml @@ -0,0 +1,136 @@ +openapi: 3.1.0 +info: + title: Familienarchiv API — Profile picture upload + version: 0.0.1-SNAPSHOT + description: > + Design-time contract for the avatar feature (.specify/features/_example). + Source of truth once shipped is the generated /v3/api-docs. +servers: + - url: http://localhost:8080 + description: Local backend (dev profile) + - url: https://archiv.raddatz.cloud + description: Production (behind Caddy) +components: + securitySchemes: + cookieAuth: + type: apiKey + in: cookie + name: SESSION + schemas: + ErrorResponse: + type: object + required: [code, message] + properties: + code: + type: string + example: AVATAR_TOO_LARGE + message: + type: string + UserProfileView: + type: object + required: [id, displayName] + properties: + id: + type: string + format: uuid + displayName: + type: string + avatarUrl: + type: [string, "null"] + description: Authenticated proxy path (/api/users/{id}/avatar) when an avatar exists, else null. + example: /api/users/3f1c.../avatar +security: + - cookieAuth: [] +paths: + /api/users/me/avatar: + post: + summary: Upload or replace the current user's avatar + operationId: uploadMyAvatar + security: + - cookieAuth: [] + requestBody: + required: true + content: + multipart/form-data: + schema: + type: object + required: [file] + properties: + file: + type: string + format: binary + description: PNG or JPEG, max 2 MB. + responses: + '200': + description: Avatar stored; updated profile returned. + content: + application/json: + schema: { $ref: '#/components/schemas/UserProfileView' } + '400': + description: Unsupported type (UNSUPPORTED_FILE_TYPE) or too large (AVATAR_TOO_LARGE). + content: + application/json: + schema: { $ref: '#/components/schemas/ErrorResponse' } + '401': + description: Unauthenticated (UNAUTHORIZED). + content: + application/json: + schema: { $ref: '#/components/schemas/ErrorResponse' } + delete: + summary: Remove the current user's avatar + operationId: deleteMyAvatar + security: + - cookieAuth: [] + responses: + '200': + description: Avatar removed; profile returned with avatarUrl null. + content: + application/json: + schema: { $ref: '#/components/schemas/UserProfileView' } + '401': + description: Unauthenticated (UNAUTHORIZED). + content: + application/json: + schema: { $ref: '#/components/schemas/ErrorResponse' } + /api/users/{id}/avatar: + get: + summary: Stream a user's avatar image (authenticated proxy) + operationId: getUserAvatar + security: + - cookieAuth: [] + parameters: + - name: id + in: path + required: true + schema: { type: string, format: uuid } + responses: + '200': + description: Image bytes. + content: + image/png: { schema: { type: string, format: binary } } + image/jpeg: { schema: { type: string, format: binary } } + '401': { description: Unauthenticated, content: { application/json: { schema: { $ref: '#/components/schemas/ErrorResponse' } } } } + '404': { description: User has no avatar, content: { application/json: { schema: { $ref: '#/components/schemas/ErrorResponse' } } } } + delete: + summary: Remove another user's avatar (admin only) + operationId: deleteUserAvatar + description: Requires Permission.ADMIN_USER (enforced by @RequirePermission on the controller). + security: + - cookieAuth: [] + parameters: + - name: id + in: path + required: true + schema: { type: string, format: uuid } + responses: + '200': + description: Avatar removed. + content: + application/json: + schema: { $ref: '#/components/schemas/UserProfileView' } + '401': { description: Unauthenticated, content: { application/json: { schema: { $ref: '#/components/schemas/ErrorResponse' } } } } + '403': + description: Caller lacks ADMIN_USER (FORBIDDEN). + content: + application/json: + schema: { $ref: '#/components/schemas/ErrorResponse' } diff --git a/.specify/features/_example/checklist-results.md b/.specify/features/_example/checklist-results.md new file mode 100644 index 00000000..d97fe1a8 --- /dev/null +++ b/.specify/features/_example/checklist-results.md @@ -0,0 +1,76 @@ +# Persona Review Results — Profile picture upload + +> Captured from the six persona spec reviews (the comments that, in a real feature, are +> posted on the Gitea issue). This is the worked example of what a completed review round +> looks like. All personas APPROVE; the two findings raised were folded into the spec +> before approval. + +## Summary + +| Persona | Verdict | Blocking FAILs | Notes | +|---|---|---|---| +| Requirements Engineer | APPROVE | none | — | +| Developer | APPROVE | none | — | +| Security | APPROVE | none (2 resolved) | See F-SEC-1, F-SEC-2 | +| DevOps | APPROVE | none | — | +| UI/UX | APPROVE | none (1 resolved) | See F-UX-1 | +| Architect | APPROVE | none (1 resolved) | See F-ARCH-1 | + +--- + +## ### Security — Spec Review + +| # | Item | Status | Note | +|---|---|---|---| +| 1 | All mutating endpoints have authn + authz `If` clauses | PASS | REQ-006 (401), REQ-009 (403) | +| 2 | Each mutating endpoint names least-privilege `Permission` | PASS | `me` = authenticated; `{id}` = ADMIN_USER | +| 3 | Audit fields server-set, forbidden in body | PASS | `avatarObjectKey` server-set (design.md) | +| 4 | IDOR surfaces addressed | PASS | `/{id}` gated by ADMIN_USER + ownership | +| 5 | Untrusted content rendered safely | PASS | image bytes via proxy + `nosniff` | +| 6 | Upload: type allow-list + size + bytes | PASS | REQ-007 (PNG/JPEG), REQ-008 (2 MB) | +| 7 | No entity internals leaked | PASS | `UserProfileView`, not `AppUser` | +| 8 | Conflicts → 409 not raw 500 | N/A | no optimistic-lock surface here | +| 9 | threat-model.md present & STRIDE-complete | PASS | [threat-model.md](./threat-model.md) | +| 10 | ASTRIDE if AI tool used | N/A | no AI agent | +| 11 | Secrets from env only | PASS | none introduced | +| 12 | Logs PII-free | PASS | user UUID only | +| 13 | New dependency has ADR + clean audit | N/A | no new dependency | + +**F-SEC-1 (resolved):** initial draft exposed a public S3 URL for `avatarUrl` → +information disclosure. Resolved: authenticated proxy `GET /api/users/{id}/avatar`. +**F-SEC-2 (resolved):** initial draft bound `avatarObjectKey` from the request body → +mass-assignment. Resolved: server-set only. +**Verdict: APPROVE.** + +## ### UI/UX — Spec Review + +| # | Item | Status | Note | +|---|---|---|---| +| 1 | Every interaction state described | PASS | idle/preview/uploading/error/done (T-10) | +| 2 | Strings via Paraglide i18n | PASS | T-8 | +| 3 | Reuses design tokens/components | PASS | placeholder uses existing initials pattern | +| 4 | Responsive per device split | PASS | control usable on phone + laptop | +| 5 | Errors via `getErrorMessage(code)` | PASS | UNSUPPORTED_FILE_TYPE / AVATAR_TOO_LARGE | +| 6 | Keyboard + screen-reader | PASS | labelled file input, alt text on image | +| 7 | Acceptance criteria measurable | PASS | sizes, status codes | +| 8 | E2E scenario per journey | PASS | T-12 | +| 9 | Confirmation for destructive action | PASS | remove asks to confirm | +| 10 | Safe rendering + image dims | PASS | fixed dims avoid layout shift | +| 11 | Live routes verified | PASS | `/profile`, `/users/[id]` exist | +| 12 | Token theming respected | PASS | semantic tokens | + +**F-UX-1 (resolved):** no loading state in first draft → spinner during upload added (REQ-... covered by state set in T-10). +**Verdict: APPROVE.** + +## ### Architect — Spec Review + +Key items PASS. **F-ARCH-1 (resolved):** bucket choice was undocumented → captured in +[adr-001-avatars-reuse-archive-bucket.md](./adr-001-avatars-reuse-archive-bucket.md). No new +domain, no boundary crossing, Person/AppUser separation intact. **Verdict: APPROVE.** + +## ### Requirements Engineer / Developer / DevOps — Spec Review + +All checklist items PASS (see each persona's checklist in `.specify/personas/`). RE: 9 REQ +ids, all EARS-formed, every limit has an `If`. Developer: reuses `FileService`/`UserService`, +`AVATAR_TOO_LARGE` four-site update is T-1. DevOps: V78 forward-only + rollback note, no new +bucket/env var, idempotent overwrite. **All three: APPROVE.** diff --git a/.specify/features/_example/design.md b/.specify/features/_example/design.md new file mode 100644 index 00000000..08816b48 --- /dev/null +++ b/.specify/features/_example/design.md @@ -0,0 +1,63 @@ +# Design — Profile picture upload + +> Companion to [`./spec.md`](./spec.md). The spec says *what*; this says *how*, and records +> the alternatives weighed for the non-obvious choices. + +## Component overview + +``` +ProfileSettings.svelte ──► +page.server.ts (form action) + (preview, validate) │ POST /api/users/me/avatar (multipart) + ▼ + UserAvatarController ── @RequirePermission(authenticated) + │ ownership/admin check for /{id} + ▼ + UserService.setAvatar(userId, MultipartFile) + │ validate type+size → ErrorCode + ├──► FileService.put("avatars/{userId}", bytes) (MinIO) + └──► userRepository.save(user.avatarObjectKey=key) + ▼ + UserProfileView { …, avatarUrl } +``` + +Reads: `GET /api/users/{id}/avatar` streams the object through the authenticated API +(`FileService.get`), so no public S3 URL is ever exposed. `avatarUrl` in the view is simply +`/api/users/{id}/avatar` when a key exists, else `null`. + +## Key decisions + +| Decision | Choice | Why | +|---|---|---| +| Where avatars live | Existing archive bucket, `avatars/{userId}` prefix | No new bucket/env var/Compose change — see [ADR-001](./adr-001-avatars-reuse-archive-bucket.md). | +| URL exposure | Authenticated proxy endpoint, not a signed/public URL | Same auth surface as the rest of the API; no key leakage (Information disclosure). | +| Object key | Deterministic `avatars/{userId}` (no random suffix) | A new upload overwrites the old object — no orphan-cleanup job needed (REQ-001). | +| `avatarObjectKey` binding | Server-set in `UserService` only | Never bound from request body — prevents pointing a user's avatar at an arbitrary object (Tampering / CWE-639). | +| Validation site | `UserService`, boundary-only | Type + size checked once, at the service boundary, mapped to `ErrorCode` (constitution §2.3). | + +## Layering & conventions + +- Controller → `UserService` only; `UserService` owns `userRepository` and calls + `FileService` (its public API), never another domain's repository. (constitution §1.2–1.3) +- New `ErrorCode.AVATAR_TOO_LARGE` requires the four-site update (see `tasks.md` T-1). +- `UserProfileView.avatarUrl` is `String` (nullable) with `@Schema` describing the proxy + path; not marked `requiredMode = REQUIRED` because it is legitimately null (REQ-004). +- After backend changes: `npm run generate:api` regenerates `avatarUrl` into the TS types. + +## Non-functional notes + +- Size cap (2 MB, REQ-008) is enforced **before** the object touches MinIO — the multipart + is read into a bounded buffer; Spring's `spring.servlet.multipart.max-file-size` is set to + a matching ceiling so an oversized body is rejected at the container edge too. +- No N+1 risk: the profile view derives `avatarUrl` from the already-loaded `avatarObjectKey` + column; no extra query, no S3 round-trip on list/read paths. +- The proxy `GET` streams bytes (no full-buffer) and sets a short `Cache-Control` so an + updated avatar propagates quickly. + +## Test strategy (maps to tasks.md) + +| Level | What | Tooling | +|---|---|---| +| Unit | `UserService.setAvatar` validation + storage interactions | JUnit + Mockito (mock `FileService`) | +| Slice | controller auth, status codes, error codes | `@WebMvcTest` | +| E2E | upload → preview → confirm → avatar visible; remove → initials | Playwright | +| Component | initials placeholder when `avatarUrl` is null | `vitest-browser-svelte` (`*.svelte.spec.ts`) | diff --git a/.specify/features/_example/spec.md b/.specify/features/_example/spec.md new file mode 100644 index 00000000..e1acce3c --- /dev/null +++ b/.specify/features/_example/spec.md @@ -0,0 +1,118 @@ +# As a user I want to upload a profile picture so other family members recognise me + +> **This is the canonical worked example for SDD in this repo.** It is fictional but +> realistic, chosen because no real avatar feature exists in the codebase. Use it as the +> reference shape for a real `spec.md`. Every section is filled — no placeholders. + +## Context & Why + +Readers and transcribers collaborate in threads and on document comments, but every user is +currently represented by initials only. Letting a user upload a small profile picture makes +the activity feed, comments, and the public user profile page (`/users/[id]`) more personal +and easier to scan — directly serving the family-archive product goal of feeling like a +shared family space, not a database. + +Constitution principles this feature depends on: +- [§2 Security Defaults](../../constitution.md#2-security-defaults) — upload validation, permission gating, no PII in logs. +- [§1.3 services own their repository](../../constitution.md#1-architecture-principles) — avatar storage goes through `UserService` + `FileService`, not a controller. +- [§3.6 ErrorCode four-site rule](../../constitution.md#3-code-quality-rules) — introduces `AVATAR_TOO_LARGE`. + +Related: builds on the existing `FileService` (MinIO) used by `Document` uploads. + +## User Journey + +A logged-in user opens their profile settings (`/profile`), clicks "Profilbild ändern", +selects a PNG or JPEG from their device, sees an instant preview, and confirms. The picture +replaces their initials everywhere their name appears. They can later remove it and fall +back to initials. An admin (with `ADMIN_USER`) can remove an inappropriate picture from +another user's account from the admin user view. + +## Requirements + +- **REQ-001** (Ubiquitous) — The user service shall store each profile picture as a single object in the existing archive bucket under the key `avatars/{userId}`, overwriting any previous object for that user. +- **REQ-002** (Event-driven) — When an authenticated user sends `POST /api/users/me/avatar` with a valid image, the user service shall store the image, set the user's `avatarObjectKey`, and return the updated profile view including a non-null `avatarUrl`. +- **REQ-003** (Event-driven) — When an authenticated user sends `DELETE /api/users/me/avatar`, the user service shall delete the stored object, clear `avatarObjectKey`, and return the profile view with `avatarUrl = null`. +- **REQ-004** (State-driven) — While a user has no stored avatar, the profile view for that user shall return `avatarUrl = null` and the frontend shall render the initials placeholder. +- **REQ-005** (Optional-feature) — Where the caller holds `Permission.ADMIN_USER`, the user service shall allow `DELETE /api/users/{id}/avatar` to remove another user's avatar. +- **REQ-006** (Unwanted-behavior) — If the request to any avatar endpoint is unauthenticated, then the system shall return `401` with `ErrorCode.UNAUTHORIZED` and store or delete nothing. +- **REQ-007** (Unwanted-behavior) — If the uploaded file's content type is not `image/png` or `image/jpeg`, then the user service shall return `400 ErrorCode.UNSUPPORTED_FILE_TYPE` and store nothing. +- **REQ-008** (Unwanted-behavior) — If the uploaded file exceeds 2 MB, then the user service shall return `400 ErrorCode.AVATAR_TOO_LARGE` and store nothing. +- **REQ-009** (Unwanted-behavior) — If a caller without `Permission.ADMIN_USER` targets another user's avatar via `/api/users/{id}/avatar`, then the system shall return `403 ErrorCode.FORBIDDEN` and modify nothing. + +## Acceptance Criteria + +- **REQ-001** — After a successful upload, exactly one object exists at `avatars/{userId}`; a second upload leaves exactly one object (no orphan), verified by a `FileService` interaction test. +- **REQ-002** — `POST /api/users/me/avatar` with a 100 KB PNG returns `200` and a body whose `avatarUrl` is a non-null string; the persisted `app_users.avatar_object_key` equals `avatars/{userId}`. +- **REQ-003** — `DELETE /api/users/me/avatar` returns `200`, the object is gone, and the response `avatarUrl` is `null`. +- **REQ-004** — `GET` profile view for a user with `avatar_object_key IS NULL` returns `avatarUrl: null`; the rendered component shows a 2-letter initials placeholder (Playwright). +- **REQ-005** — An `ADMIN_USER` caller deleting another user's avatar returns `200`; the target's `avatar_object_key` becomes `NULL`. +- **REQ-006** — An unauthenticated `POST`/`DELETE` returns `401`; bucket object count is unchanged. +- **REQ-007** — A `text/plain` or `application/pdf` upload returns `400 UNSUPPORTED_FILE_TYPE`; bucket object count is unchanged. +- **REQ-008** — A 2.1 MB PNG returns `400 AVATAR_TOO_LARGE`; bucket object count is unchanged. +- **REQ-009** — A non-admin caller targeting another user's id returns `403 FORBIDDEN`; the target's `avatar_object_key` is unchanged. + +## Out of Scope + +- Image cropping, resizing, or transformation — the client sends a final image; the server stores it verbatim within the size limit. +- Avatars for historical `Person` entities — this feature is for `AppUser` accounts only (Person ≠ AppUser). +- Gravatar / external avatar providers. +- Animated formats (GIF/WebP) — PNG and JPEG only in v1. + +## API / Contract Stub + +See [`./api-contract.yaml`](./api-contract.yaml). Endpoints: +`POST /api/users/me/avatar` (multipart), `DELETE /api/users/me/avatar`, +`DELETE /api/users/{id}/avatar` (ADMIN_USER). The profile view gains an optional +`avatarUrl: string | null`. All mutating endpoints carry `@RequirePermission` — `me` +endpoints require an authenticated session; the `{id}` delete requires `ADMIN_USER`. + +## Data Model Changes + +- Add nullable `avatar_object_key VARCHAR(512)` to `app_users`. +- Flyway `V78__add_app_user_avatar_object_key.sql` (next free number — verify against + `backend/src/main/resources/db/migration/` on disk before committing). +- **Rollback:** forward-only. Reverse manually with `ALTER TABLE app_users DROP COLUMN avatar_object_key;`. The MinIO `avatars/` objects are orphaned but harmless on rollback and can be pruned with `mc rm --recursive`. + +## Security Considerations + +STRIDE categories touched: **Tampering** (mass-assignment of `avatarObjectKey` if bound from +body), **Elevation of privilege** (a non-admin modifying another user's avatar — REQ-009), +**Denial of service** (oversized upload — REQ-008), **Information disclosure** (avatar URL +must not expose a signed key that bypasses auth). No AI agent involved, so ASTRIDE does not +apply. Full analysis: [`./threat-model.md`](./threat-model.md). + +## Open Questions + +> All resolved before implementation. + +- [x] Public or signed avatar URL? — **Resolved:** served through an authenticated + `GET /api/users/{id}/avatar` proxy (same auth as the rest of the API), not a public S3 URL. +- [x] New bucket or reuse archive bucket? — **Resolved:** reuse the archive bucket under an + `avatars/` prefix; see [`./adr-001-avatars-reuse-archive-bucket.md`](./adr-001-avatars-reuse-archive-bucket.md). + +## Traceability + +| REQ-ID | Task ID(s) | Test ID(s) | Status | +|---|---|---|---| +| REQ-001 | T-3 | `UserServiceAvatarTest#storesUnderUserKey`, `…#replaceLeavesNoOrphan` | Planned | +| REQ-002 | T-4 | `UserAvatarControllerTest#uploadReturnsAvatarUrl` | Planned | +| REQ-003 | T-5 | `UserAvatarControllerTest#deleteClearsAvatar` | Planned | +| REQ-004 | T-7 | `avatar-placeholder.svelte.spec.ts` | Planned | +| REQ-005 | T-6 | `UserAvatarControllerTest#adminDeletesOthersAvatar` | Planned | +| REQ-006 | T-2 | `UserAvatarControllerTest#unauthenticatedReturns401` | Planned | +| REQ-007 | T-2 | `UserAvatarControllerTest#rejectsNonImage` | Planned | +| REQ-008 | T-2 | `UserAvatarControllerTest#rejectsOversize` | Planned | +| REQ-009 | T-6 | `UserAvatarControllerTest#nonAdminForbiddenOnOthers` | Planned | + +Mirrored in [`.specify/rtm.md`](../../rtm.md). + +## Persona Review Results + +| Persona | Status | Key Findings | Resolved | +|---|---|---|---| +| Requirements Engineer | APPROVE | All 9 REQ ids EARS-formed; every limit has an `If` clause. | — | +| Developer | APPROVE | Reuses `FileService`/`UserService`; `AVATAR_TOO_LARGE` four-site update listed (T-1). | — | +| Security | APPROVE | REQ-006/008/009 cover authn/DoS/EoP; `avatarObjectKey` server-set only (see threat model T-1). | Yes | +| DevOps | APPROVE | V78 forward-only with rollback note; no new bucket/env var. | — | +| UI/UX | APPROVE | Placeholder + loading/error states specified; strings via i18n (T-8). | — | +| Architect | APPROVE | Bucket-reuse decision captured in ADR-001; no new domain, no boundary crossing. | Yes | diff --git a/.specify/features/_example/tasks.md b/.specify/features/_example/tasks.md new file mode 100644 index 00000000..b43e800e --- /dev/null +++ b/.specify/features/_example/tasks.md @@ -0,0 +1,47 @@ +# Tasks — Profile picture upload + +> Red/Green TDD order: each implementation task is preceded by the failing test that +> requires it. Task IDs are referenced from `spec.md` → Traceability and from `.specify/rtm.md`. +> Check off as work lands; reference the issue in each commit (`Refs #`). + +## Backend + +- [ ] **T-1** Add `ErrorCode.AVATAR_TOO_LARGE` in all four sites at once: `ErrorCode.java`, + `frontend/src/lib/shared/errors.ts`, `getErrorMessage()`, `messages/{de,en,es}.json`. + *(No new behavior yet — enables REQ-008's error.)* → covers REQ-008 (error plumbing) +- [ ] **T-2** `@WebMvcTest` `UserAvatarControllerTest`: write failing slice tests — + `unauthenticatedReturns401`, `rejectsNonImage` (400 UNSUPPORTED_FILE_TYPE), + `rejectsOversize` (400 AVATAR_TOO_LARGE). Then implement `UserAvatarController` + + `@RequirePermission` to green. → REQ-006, REQ-007, REQ-008 +- [ ] **T-3** Unit `UserServiceAvatarTest`: failing tests `storesUnderUserKey`, + `replaceLeavesNoOrphan`, validation maps to `DomainException`. Then implement + `UserService.setAvatar`/`removeAvatar` (mock `FileService`) to green. → REQ-001, REQ-002, REQ-003 +- [ ] **T-4** Flyway `V78__add_app_user_avatar_object_key.sql` (verify next free number on + disk) adding nullable `avatar_object_key VARCHAR(512)`; add the column + `@Schema` to + `AppUser` / `UserProfileView` (`avatarUrl` derived). Test: repository round-trip. → REQ-002 +- [ ] **T-5** `deleteMyAvatar` controller test + impl (clears key, deletes object, returns + `avatarUrl: null`). → REQ-003 +- [ ] **T-6** Admin path: failing tests `adminDeletesOthersAvatar` (200), + `nonAdminForbiddenOnOthers` (403). Implement ownership/`ADMIN_USER` check to green. → REQ-005, REQ-009 +- [ ] **T-7** Authenticated proxy `getUserAvatar` streaming endpoint + `Content-Type` + + `X-Content-Type-Options: nosniff`; test 200 bytes / 404 when no avatar. → REQ-004 (view side) +- [ ] **T-A** Run `npm run generate:api` after T-4/T-7 so `avatarUrl` lands in `api.ts`. + +## Frontend + +- [ ] **T-8** i18n keys for the new strings in `messages/{de,en,es}.json` (button labels, + validation errors mapped via `getErrorMessage`). → REQ-007, REQ-008 (UX) +- [ ] **T-9** Component test `avatar-placeholder.svelte.spec.ts`: failing test asserting + initials render when `avatarUrl` is null; implement the placeholder. → REQ-004 +- [ ] **T-10** `/profile` upload control: file picker, client-side type/size pre-check, + instant preview, confirm/remove. States: idle/preview/uploading/error/done. → REQ-002, REQ-003 +- [ ] **T-11** Render avatar where names appear (comments, activity feed, `/users/[id]`), + falling back to the placeholder. → REQ-004 +- [ ] **T-12** E2E `avatar.spec.ts`: upload → preview → confirm → avatar visible; remove → + initials return. → REQ-002, REQ-003, REQ-004 + +## Cross-cutting + +- [ ] **T-13** Set `spring.servlet.multipart.max-file-size` to a 2 MB-matching ceiling so an + oversized body is rejected at the container edge (defense in depth for REQ-008). +- [ ] **T-14** Update `.specify/rtm.md` Status column to `Done` per REQ as each test goes green. diff --git a/.specify/features/_example/threat-model.md b/.specify/features/_example/threat-model.md new file mode 100644 index 00000000..670135a7 --- /dev/null +++ b/.specify/features/_example/threat-model.md @@ -0,0 +1,45 @@ +# Threat Model — Profile picture upload + +**Feature spec:** [./spec.md](./spec.md) +**Date:** 2026-06-13 +**Author:** Security persona (worked example) + +## Data Flow Diagram (text) + +**Actors** +- Anonymous visitor (unauthenticated) +- Authenticated user (uploads their own avatar) +- Admin (`Permission.ADMIN_USER` — may remove others' avatars) + +**Trust boundaries** +- TB-1: Browser ⇄ Caddy (public internet ⇄ DMZ) +- TB-2: Caddy ⇄ Backend `:8080` (DMZ ⇄ app) +- TB-3: Backend ⇄ MinIO + PostgreSQL (app ⇄ data plane) + +**Data flows** +- F-1: Browser → [TB-1,TB-2] → `UserAvatarController` : multipart image +- F-2: `UserService` → [TB-3] → MinIO : object at `avatars/{userId}` +- F-3: `UserService` → [TB-3] → PostgreSQL : `app_users.avatar_object_key` +- F-4: Browser → [TB-1,TB-2,TB-3] → MinIO (via proxy GET) : image bytes + +## STRIDE + +| Threat Category | Asset / Flow | Threat Description | Mitigation | Likelihood × Impact | Status | +|---|---|---|---|---|---| +| **S**poofing | F-1 | Unauthenticated caller uploads/deletes an avatar | Session auth required; `@RequirePermission` (REQ-006) | Low × Med | Mitigated | +| **T**ampering | F-3 | Caller sets `avatarObjectKey` via request body to point at an arbitrary stored object | `avatarObjectKey` is server-set in `UserService` only, never bound from body (CWE-639) | Med × High | Mitigated | +| **R**epudiation | F-2/F-3 | No record of who changed an avatar | Standard request logging by user UUID (no PII); admin deletions auditable via existing logs | Low × Low | Accepted | +| **I**nformation disclosure | F-4 | A public/signed S3 URL would let anyone fetch any avatar without auth | Avatars served only through the authenticated proxy `GET /api/users/{id}/avatar`; no public URL | Med × Med | Mitigated | +| **I**nformation disclosure | F-1 | Malicious file (polyglot) served back with a sniffed content type → stored XSS | Store with a fixed `image/png`/`image/jpeg` content type; proxy sets `Content-Type` + `X-Content-Type-Options: nosniff`; only PNG/JPEG accepted (REQ-007) | Low × High | Mitigated | +| **D**enial of service | F-1/F-2 | Oversized or many uploads exhaust storage/memory | 2 MB cap enforced before MinIO write + `multipart.max-file-size` ceiling (REQ-008); deterministic key means one object per user | Med × Med | Mitigated | +| **E**levation of privilege | F-1 | Non-admin removes/replaces another user's avatar via `/{id}` | Ownership check; `ADMIN_USER` required for `/{id}` (REQ-005/REQ-009, 403) | Low × Med | Mitigated | + +## ASTRIDE + +Not applicable — this feature invokes no AI agent, model, or tool. + +## Residual Risk + +- **Repudiation (Accepted):** avatar changes are not written to a dedicated audit table. + Accepted because the asset is low-value (a self-chosen picture) and request logs already + attribute the action to a user UUID. Revisit if avatars ever become trust signals. diff --git a/.specify/personas/architect.md b/.specify/personas/architect.md new file mode 100644 index 00000000..38567878 --- /dev/null +++ b/.specify/personas/architect.md @@ -0,0 +1,40 @@ +# Persona — Architect (spec review) + +> Concise spec-review checklist. Full character persona: +> [`.claude/personas/architect.md`](../../.claude/personas/architect.md). This file gates a +> `spec.md` and its `design.md`/ADRs for systemic fit and long-term consequence. + +## Role summary + +I check that a feature fits the system's domain boundaries and decision history, and that +any irreversible choice it makes is captured in an ADR before code is written. I block specs +that quietly contradict an Accepted ADR, blur a domain boundary, or bake in a decision with +no recorded rationale. + +## Review checklist (PASS / FAIL / QUESTION per item) + +1. Does the feature respect the package-by-domain structure — new code in the right domain, no logic smeared across layer packages? +2. Does it honor the layering rule and the frontend boundary rule, or does it justify and record any new cross-domain edge? +3. Does any irreversible or contentious decision (new dependency, new domain, data-model shape, response-as-view vs entity, sync vs async side effect) have an ADR in `Proposed`/`Accepted` status under `docs/adr/`? +4. Does the spec contradict any existing Accepted ADR — and if a change is intended, does it **supersede** that ADR rather than silently diverge? +5. Is the ADR number the next free one verified against `docs/adr/` on disk? +6. Does the design reuse an established pattern (in-transaction views per ADR-036, domain events per ADR-006, DatePrecision sharing per ADR-039/040) instead of a novel mechanism for a solved problem? +7. Are domain terms used per [docs/GLOSSARY.md](../../docs/GLOSSARY.md), keeping the ubiquitous language consistent? +8. Is the blast radius bounded — does the change avoid forcing edits across unrelated domains, or is the coupling explicitly justified? +9. Does the data model choose the right precision/constraint level deliberately (e.g. NOT NULL audit fields, CHECK constraints) rather than by default, and is the choice recorded? +10. Does the spec keep `Person`/`AppUser` (and other established separations) distinct? +11. Are non-functional consequences (performance of the lazy-fetch path, N+1 risk, index needs) named in `design.md`? +12. Does `design.md` list the alternatives considered and why they were rejected, not just the chosen path? + +## EARS patterns to watch for + +- **Ubiquitous** requirements (`The shall `) encode architectural invariants — confirm each invariant is enforced at the right layer (DB CHECK, service guard, or type) and not merely asserted in prose. +- **Optional-feature** requirements signal a new seam/extension point — verify it does not become an unbounded plugin surface without an ADR. +- Watch for requirements that imply a second source of truth for data that already has an owning domain. + +## Output format + +A Gitea comment titled **`### Architect — Spec Review`** with the checklist table +`| # | Item | Status | Note |`, then `Verdict: APPROVE` / `CHANGES REQUESTED` listing +blocking `FAIL` numbers and, for any decision lacking one, the specific ADR that must be +written before implementation. diff --git a/.specify/personas/developer.md b/.specify/personas/developer.md new file mode 100644 index 00000000..94481b72 --- /dev/null +++ b/.specify/personas/developer.md @@ -0,0 +1,39 @@ +# Persona — Developer (spec review) + +> Concise spec-review checklist. Full character persona: +> [`.claude/personas/developer.md`](../../.claude/personas/developer.md). This file gates a +> `spec.md` for implementability against the real codebase. + +## Role summary + +I check that a spec can actually be built in *this* codebase without fighting its +architecture: that it reuses existing services, layers, and error machinery, and that its +requirements decompose cleanly into red/green TDD tasks. I block specs that invent parallel +structures or hand-wave the hard integration points. + +## Review checklist (PASS / FAIL / QUESTION per item) + +1. Does the spec reference existing service interfaces (e.g. `DocumentService`, `FileService`, `UserService`) rather than inventing new ones inconsistent with the current layer structure? +2. Does it respect the layering rule — no requirement implies a controller touching a repository or a service reaching into another domain's repository? +3. If it adds a backend domain, does it commit to adding the package to `ArchitectureTest`'s allow-lists? +4. Are new error conditions expressed as named `ErrorCode`s, with the four-site update (`ErrorCode.java`, `errors.ts`, `getErrorMessage()`, `messages/{de,en,es}.json`) called out as tasks? +5. Does every entity/DTO field the spec adds get `@Schema(requiredMode = REQUIRED)` where always-populated, and is `npm run generate:api` listed as a task after backend changes? +6. Are frontend changes inside the correct `$lib//` boundary, with any cross-domain import either pre-allowed in `eslint.config.js` or flagged for an explicit allow-entry? +7. Does each `REQ-NNN` map to a concrete test at the right level (unit / `@WebMvcTest` slice / Playwright E2E per COLLABORATING.md's table) in `tasks.md`? +8. Is lazy-loading handled — does any returned entity with a lazy collection get a view (ADR-036) instead of being serialized raw? +9. Does the design avoid premature abstraction (KISS over DRY) — no new base class/util introduced before a third caller exists? +10. Are data-model changes expressed as a single forward-only Flyway migration with the next free `V` number verified against disk? +11. Does the spec avoid backwards-compat shims for code paths that have no existing callers? +12. Is the `tasks.md` decomposition red/green-ordered — a failing test task precedes each implementation task? + +## EARS patterns to watch for + +- **Event-driven** requirements must name the exact endpoint/method so the test target is unambiguous (`When POST /api/users/{id}/avatar receives a valid image, the user service shall …`). +- **Unwanted-behavior** requirements are the ones that become `@WebMvcTest` error-path cases — flag any that lack a stated `ErrorCode` and HTTP status. +- **Optional-feature** (`Where …`) requirements map to a `@RequirePermission` gate — confirm the permission already exists or is added. + +## Output format + +A Gitea comment titled **`### Developer — Spec Review`** with the checklist table +`| # | Item | Status | Note |`, then `Verdict: APPROVE` / `CHANGES REQUESTED` listing the +blocking `FAIL` numbers and the single most important integration risk in one sentence. diff --git a/.specify/personas/devops.md b/.specify/personas/devops.md new file mode 100644 index 00000000..42b5cb3a --- /dev/null +++ b/.specify/personas/devops.md @@ -0,0 +1,39 @@ +# Persona — DevOps (spec review) + +> Concise spec-review checklist. Full character persona: +> [`.claude/personas/devops.md`](../../.claude/personas/devops.md). This file gates a +> `spec.md` for deployability, migration safety, and CI/observability impact. + +## Role summary + +I check that a feature can ship to the self-hosted Gitea-Actions / Docker-Compose +environment without breaking deploys, migrations, or observability. I block specs that add +a migration with no rollback story, a new env var nobody documented, or a CI step that the +act_runner cannot execute. + +## Review checklist (PASS / FAIL / QUESTION per item) + +1. Does the spec include a rollback strategy for any database migration it introduces (forward-only `V` plus the manual DDL to reverse it, or an explicit "no rollback, forward-fix only" statement)? +2. Is the Flyway migration number the next free `V` verified against disk, not copied from a stale issue body? +3. Are all new configuration values introduced as documented env vars (added to `.env.example`) and read via env, never hard-coded? +4. Does any new CI step avoid `actions/(upload|download)-artifact@v4+` and other features the Gitea `act_runner` does not support? +5. If the spec adds a CI guard, is it self-testing (the regex proves it catches the bad form and ignores the good form), matching the existing guard style? +6. Does the feature keep the management port (`8081`) / app port (`8080`) separation intact, and not require Caddy to proxy `/actuator/*`? +7. Are new dependencies pinned, and does the change keep `npm audit --audit-level=high` and Semgrep green? +8. Does a new external service or sidecar come with a healthcheck and a documented Compose entry, and is bucket/bootstrap logic idempotent (re-deploy must not fail)? +9. Are new metrics/logs/traces routed through the existing observability stack (Prometheus scrape, Promtail/Loki, Tempo, GlitchTip) rather than a new ad-hoc channel? +10. Does logging added by the feature stay PII-free and structured (JSON), consistent with the existing log pipeline? +11. Is the feature backwards-compatible across a rolling deploy, or does the spec state the required downtime/ordering (migrate-then-deploy)? +12. Does the spec avoid committing secrets, and does any composite-action secret flow follow the unquoted-heredoc env convention (ADR-029)? + +## EARS patterns to watch for + +- **State-driven** (`While a migration is in progress, the system shall …`) and **Unwanted-behavior** (`If the OCR service is unavailable, then the system shall return OCR_SERVICE_UNAVAILABLE`) requirements encode operational resilience — flag mutating/processing features that lack them. +- **Optional-feature** (`Where the observability stack is enabled …`) requirements gate optional infra — confirm the feature degrades cleanly when it is off. + +## Output format + +A Gitea comment titled **`### DevOps — Spec Review`** with the checklist table +`| # | Item | Status | Note |`, then `Verdict: APPROVE` / `CHANGES REQUESTED` listing +blocking `FAIL` numbers, with the migration/rollback line called out explicitly when +relevant. diff --git a/.specify/personas/requirements-engineer.md b/.specify/personas/requirements-engineer.md new file mode 100644 index 00000000..624e1b6e --- /dev/null +++ b/.specify/personas/requirements-engineer.md @@ -0,0 +1,43 @@ +# Persona — Requirements Engineer (spec review) + +> Concise spec-review checklist. The full character persona (used for issue/PR review via +> the `review-issue` / `review-pr` skills) lives at +> [`.claude/personas/req_engineer.md`](../../.claude/personas/req_engineer.md). This file is +> scoped to one job: gate a `spec.md` before implementation starts. + +## Role summary + +I own requirement quality: every requirement must be atomic, testable, uniquely identified, +and written in EARS so an engineer and an AI agent read it the same way. I block specs that +are ambiguous, unmeasurable, or untraceable — vague requirements become vague code. + +## Review checklist (PASS / FAIL / QUESTION per item) + +1. Does every requirement have a unique zero-padded `REQ-NNN` ID, scoped to this feature? +2. Is every requirement written in one of the five EARS patterns (no free-prose "shall" sentences)? +3. Is each requirement atomic — exactly one testable behavior, no "and"-joined clauses hiding two requirements? +4. Does every requirement name a concrete system actor (e.g. `the document service`, `the upload form`) rather than a vague "system"? +5. Does each `REQ-NNN` have at least one matching, **measurable** acceptance criterion (numbers/limits, not adjectives like "fast" or "user-friendly")? +6. Are all five EARS patterns considered, and is each used where appropriate (not every requirement forced into Ubiquitous)? +7. Is there an Unwanted-behavior (`If …`) requirement for every error, limit, and rejected input the happy path implies? +8. Does the `## Out of Scope` section explicitly fence off the nearest tempting scope creep? +9. Are all `## Open Questions` resolved (or explicitly deferred with an owner) — none left as silent blockers? +10. Does the spec link the constitution principle(s) it depends on in `## Context & Why`? +11. Is every `REQ-NNN` present in `.specify/rtm.md` with a Feature, Test, and Status column filled (even if Status = Planned)? +12. Does the spec reuse existing domain vocabulary from [docs/GLOSSARY.md](../../docs/GLOSSARY.md) (e.g. Person vs AppUser, Chronik vs Aktivität) rather than inventing terms? +13. Are the User Journey and E2E Scenarios (per COLLABORATING.md) present and consistent with the EARS requirements? + +## EARS patterns to watch for (common violations) + +- **Ubiquitous** — `The shall .` Violation: an invariant written as prose with no "shall". +- **Event-driven** — `When , the shall .` Violation: a trigger described but the response left implicit. +- **State-driven** — `While , the shall .` Violation: a state precondition buried inside an Event-driven clause. +- **Optional-feature** — `Where , the shall .` Violation: a permission-/flag-gated behavior written as Ubiquitous, so it appears mandatory. +- **Unwanted-behavior** — `If , then the shall .` Violation: missing entirely — the single most common gap. Every limit and rejected input needs one. + +## Output format + +A Gitea comment titled **`### Requirements Engineer — Spec Review`** containing the +checklist as a table `| # | Item | Status | Note |` with `PASS` / `FAIL` / `QUESTION` per +row, then a short verdict line: `Verdict: APPROVE` or `Verdict: CHANGES REQUESTED` with the +blocking `FAIL` numbers listed. diff --git a/.specify/personas/security.md b/.specify/personas/security.md new file mode 100644 index 00000000..067a5915 --- /dev/null +++ b/.specify/personas/security.md @@ -0,0 +1,42 @@ +# Persona — Security (spec review) + +> Concise spec-review checklist. Full character persona (Nora "NullX" Steiner): +> [`.claude/personas/security_expert.md`](../../.claude/personas/security_expert.md). This +> file gates a `spec.md` and its `threat-model.md` before implementation. + +## Role summary + +I read every spec adversarially: I assume the requirement will be hit by an unauthenticated +attacker, a logged-in user attacking another user's data, and malicious input. I block specs +whose mutating endpoints, file handling, or audit trails leave a hole that the happy-path +requirements never mention. + +## Review checklist (PASS / FAIL / QUESTION per item) + +1. Are **all** state-mutating endpoints (`POST/PUT/PATCH/DELETE`) covered by an Unwanted-behavior EARS clause for unauthenticated **and** unauthorized access, each naming the `Permission` and the response code? +2. Does every mutating endpoint name the `@RequirePermission(Permission.X)` it will carry — and is that permission the least privilege that works? +3. Are audit fields (`createdBy`/`updatedBy`) specified as server-set from the session principal, with an explicit requirement forbidding them in the request body (mass-assignment / authorship-forgery, CWE-639)? +4. Is every IDOR surface addressed — does fetching/mutating a child resource verify it belongs to the caller's accessible parent (e.g. JourneyItem → Geschichte), with a requirement and a test? +5. Is all untrusted text (user input, OCR/import-derived) specified to render via default escaping, never `{@html}` (CWE-79)? +6. For file uploads: are content-type allow-list, size limit, and magic-byte/extension validation specified as requirements with concrete numbers and an `ErrorCode`? +7. Does the spec avoid leaking entity internals (email, password hash, group graph) in any response — i.e. does it use a view, not a raw `AppUser`/entity? +8. Are concurrency conflicts (optimistic locking) specified to surface as `conflict()` (409), never a raw 500 exposing Hibernate internals (CWE-209)? +9. Does the `threat-model.md` exist and cover the relevant STRIDE categories for each new data flow and trust boundary? +10. If the feature invokes an AI agent/tool (OCR/NLP/LLM), does the threat model cover the ASTRIDE extensions (prompt injection, context poisoning, unsafe tool invocation, reasoning subversion)? +11. Are secrets (tokens, DSNs, passwords) sourced only from env vars, with none introduced into the repo, config, or logs? +12. Does logging for this feature exclude PII beyond a stable UUID (no names, emails, document/transcription content)? +13. Does a new runtime dependency (if any) have an ADR and a clean `npm audit` / Semgrep status? + +## EARS patterns to watch for + +- The **Unwanted-behavior** pattern (`If , then the shall `) is *the* security pattern. Every auth, authz, validation, and limit case must appear as one. A spec with zero `If` requirements on a mutating endpoint is an automatic `FAIL`. +- **Optional-feature** (`Where the caller has Permission.X …`) requirements encode the authorization model — verify the gate is on the *write*, not just the read. +- Watch for **Ubiquitous** requirements that quietly assume trust ("The system shall store the uploaded file") with no companion `If` clause validating it first. + +## Output format + +A Gitea comment titled **`### Security — Spec Review`** with the checklist table +`| # | Item | Status | Note |`, each `FAIL` tagged with its CWE where applicable, then +`Verdict: APPROVE` / `CHANGES REQUESTED` listing blocking `FAIL` numbers. Security `FAIL`s +are hard blockers — a spec does not proceed until each is resolved or risk-accepted in the +threat model. diff --git a/.specify/personas/ui-ux.md b/.specify/personas/ui-ux.md new file mode 100644 index 00000000..4a75c7a3 --- /dev/null +++ b/.specify/personas/ui-ux.md @@ -0,0 +1,39 @@ +# Persona — UI/UX (spec review) + +> Concise spec-review checklist. Full character persona: +> [`.claude/personas/ui_expert.md`](../../.claude/personas/ui_expert.md). This file gates a +> `spec.md` for user-facing features against the project's design system and audience split. + +## Role summary + +I check that a user-facing feature is usable by *this* audience — older transcribers on +laptops/tablets and younger readers on phones — and that it uses the established design +tokens, components, and i18n rather than reinventing them. I block specs whose UI is +described in adjectives instead of states, or that ignore accessibility and responsiveness. + +## Review checklist (PASS / FAIL / QUESTION per item) + +1. Does the spec describe every interaction **state** (loading, empty, error, success, disabled), not just the happy path? +2. Are user-facing strings specified to go through Paraglide i18n with keys added to `messages/{de,en,es}.json` — no hard-coded German/English literals? +3. Does it reuse the established component library and patterns (`BackButton`, the card pattern, `brand-navy`/`brand-mint` tokens, `font-serif`/`font-sans`) rather than introducing new one-off styles? +4. Is the responsive behavior specified per the device split — Critical for the reader/phone path, at least Minor for the author/laptop path — with concrete breakpoints, not "responsive"? +5. Are error states mapped to `getErrorMessage(code)` output so the user sees a localized message, never a raw code or stack? +6. Is every interactive element keyboard-reachable and screen-reader-labeled (the project runs `@axe-core/playwright`)? +7. Are acceptance criteria measurable (e.g. "image preview appears within 1 of selection", "tap target ≥ 44px"), not adjectival ("looks clean")? +8. Does the spec define an E2E Playwright scenario (per COLLABORATING.md) for each primary user journey step? +9. For destructive or irreversible actions, is a confirmation/undo affordance specified? +10. Does any uploaded/derived content render through default escaping (no `{@html}`), and are images given alt text / dimensions to avoid layout shift? +11. Does the feature respect existing navigation (live DOM nav, real routes — verify route names against the running app, since CLAUDE.md route lists can be stale)? +12. Is dark-mode / token theming respected (uses semantic tokens like `bg-surface`/`text-ink-3`, not raw palette constants)? + +## EARS patterns to watch for + +- **State-driven** (`While the upload is in progress, the upload form shall show a progress indicator`) requirements capture UI states — a UI spec with no `While` requirements usually means the loading/disabled states were forgotten. +- **Event-driven** (`When the user selects an image, the form shall render a preview`) requirements map directly to Playwright steps — confirm each has a measurable acceptance criterion. +- **Unwanted-behavior** (`If the selected file exceeds the size limit, then the form shall show a localized error and not upload`) requirements cover client-side validation feedback. + +## Output format + +A Gitea comment titled **`### UI/UX — Spec Review`** with the checklist table +`| # | Item | Status | Note |`, then `Verdict: APPROVE` / `CHANGES REQUESTED` listing +blocking `FAIL` numbers and the single biggest usability/accessibility gap in one sentence. diff --git a/.specify/rtm.md b/.specify/rtm.md new file mode 100644 index 00000000..c3d589b7 --- /dev/null +++ b/.specify/rtm.md @@ -0,0 +1,36 @@ +# Requirements Traceability Matrix (RTM) + +> Living document. One row per `REQ-NNN` across all in-flight and shipped features. It links +> a requirement to the design that realises it, the code that implements it, and the +> test(s) that prove it — so any requirement can be traced end to end, and any orphan +> (a requirement with no test, or a test with no requirement) is visible. + +## How to update + +1. When a `spec.md` is approved, copy its `## Traceability` rows here with `Status: Planned`. +2. As tasks land, fill `Implementation File(s)` and flip `Status` → `In progress` → `Done`. +3. `REQ-ID`s are **scoped per feature**, so always qualify with the Feature column — `REQ-001` + in *avatar* is not `REQ-001` in another feature. +4. The `sdd-gate.yml` CI job (`traceability-check`) warns (non-blocking, for now) when a + `spec.md` contains a `REQ-NNN` that does not appear in this file. Keep it in sync to keep + the warning quiet; it flips to blocking once adoption settles (see the workflow's TODO). + +## Status legend + +`Planned` · `In progress` · `Done` · `Deferred` + +## Matrix + +| REQ-ID | Requirement Summary | Feature | Design Artifact | Implementation File(s) | Test(s) | Status | +|---|---|---|---|---|---|---| +| REQ-001 | Store avatar at `avatars/{userId}`, overwrite | profile-picture-upload (_example) | features/_example/design.md; adr-001 | `UserService` (planned) | `UserServiceAvatarTest#storesUnderUserKey`, `#replaceLeavesNoOrphan` | Planned | +| REQ-002 | Upload self avatar → 200 + avatarUrl | profile-picture-upload (_example) | features/_example/design.md; api-contract.yaml | `UserAvatarController`, `UserService` (planned) | `UserAvatarControllerTest#uploadReturnsAvatarUrl` | Planned | +| REQ-003 | Delete self avatar → avatarUrl null | profile-picture-upload (_example) | features/_example/api-contract.yaml | `UserAvatarController` (planned) | `UserAvatarControllerTest#deleteClearsAvatar` | Planned | +| REQ-004 | No avatar → null + initials placeholder | profile-picture-upload (_example) | features/_example/design.md | `UserProfileView`, avatar component (planned) | `avatar-placeholder.svelte.spec.ts` | Planned | +| REQ-005 | ADMIN_USER may delete others' avatar | profile-picture-upload (_example) | features/_example/api-contract.yaml | `UserAvatarController` (planned) | `UserAvatarControllerTest#adminDeletesOthersAvatar` | Planned | +| REQ-006 | Unauthenticated → 401, store nothing | profile-picture-upload (_example) | features/_example/threat-model.md | `SecurityConfig`, controller (planned) | `UserAvatarControllerTest#unauthenticatedReturns401` | Planned | +| REQ-007 | Non-image → 400 UNSUPPORTED_FILE_TYPE | profile-picture-upload (_example) | features/_example/design.md | `UserService` (planned) | `UserAvatarControllerTest#rejectsNonImage` | Planned | +| REQ-008 | Over 2 MB → 400 AVATAR_TOO_LARGE | profile-picture-upload (_example) | features/_example/design.md | `UserService`, `ErrorCode` (planned) | `UserAvatarControllerTest#rejectsOversize` | Planned | +| REQ-009 | Non-admin on others → 403 FORBIDDEN | profile-picture-upload (_example) | features/_example/threat-model.md | `UserAvatarController` (planned) | `UserAvatarControllerTest#nonAdminForbiddenOnOthers` | Planned | + + diff --git a/.specify/templates/adr.md b/.specify/templates/adr.md new file mode 100644 index 00000000..52621d5a --- /dev/null +++ b/.specify/templates/adr.md @@ -0,0 +1,42 @@ + + +# ADR-NNN — + +**Status:** Proposed +**Date:** +**Issue:** # + +## Context + + + +## Decision + + + +## Alternatives Considered + +| Option | Pros | Cons | Reason rejected | +|---|---|---|---| +| | | | **Chosen** | +| | | | | +| | | | | + +## Consequences + + + +## References + +- diff --git a/.specify/templates/api-contract-stub.md b/.specify/templates/api-contract-stub.md new file mode 100644 index 00000000..cf3d1f75 --- /dev/null +++ b/.specify/templates/api-contract-stub.md @@ -0,0 +1,94 @@ +# API Contract Stub + +This project is **REST + OpenAPI**. The backend serves the live spec via springdoc at +`http://localhost:8080/v3/api-docs` (dev profile only), and the frontend generates its +TypeScript client from it with `npm run generate:api` (`openapi-typescript` → +`frontend/src/lib/generated/api.ts`). There is no GraphQL in this stack. + +> **The live spec is generated from the Java controllers — it is the source of truth.** A +> hand-written contract under `.specify/features//api-contract.yaml` is a *design +> artifact*: it pins the intended shape during spec review and is checked against the +> generated spec once the endpoint exists. Keep it OpenAPI **3.1**, and keep +> `@Schema(requiredMode = REQUIRED)` on the Java side as the real driver of `required`. + +## How to use this stub + +1. Copy the skeleton below to `.specify/features//api-contract.yaml`. +2. Fill in the paths/methods/schemas your feature adds. Every mutating path documents the + `403`/`401` responses and the `cookieAuth` security requirement (matching the real + `@RequirePermission` gate). +3. The `sdd-gate.yml` CI job lints any changed `api-contract.yaml` with Spectral + (`npx @stoplight/spectral-cli lint`). Run it locally the same way before pushing. +4. After the endpoint ships, run `npm run generate:api` and diff the generated types against + this contract; reconcile any drift (the generated spec wins — update the contract). + +## OpenAPI 3.1 skeleton + +```yaml +openapi: 3.1.0 +info: + title: Familienarchiv API — + version: 0.0.1-SNAPSHOT + description: Design-time contract for . Source of truth is the generated /v3/api-docs. +servers: + - url: http://localhost:8080 + description: Local backend (dev profile) + - url: https://archiv.raddatz.cloud + description: Production (behind Caddy) +components: + securitySchemes: + cookieAuth: # Spring Session JDBC — opaque session id in the SESSION cookie + type: apiKey + in: cookie + name: SESSION + schemas: + ErrorResponse: # shape produced by GlobalExceptionHandler + type: object + required: [code, message] + properties: + code: + type: string + description: Machine-readable ErrorCode (see ErrorCode.java / errors.ts). + example: FORBIDDEN + message: + type: string + # : # always a view, never a lazy-collection entity (ADR-036) + # type: object + # required: [id] + # properties: + # id: { type: string, format: uuid } +security: + - cookieAuth: [] # default: every path requires a session unless overridden to [] +paths: + /api/: + post: + summary: + operationId: + security: + - cookieAuth: [] # plus @RequirePermission(Permission.X) on the controller + requestBody: + required: true + content: + application/json: + schema: { $ref: '#/components/schemas/' } + responses: + '201': + description: Created + content: + application/json: + schema: { $ref: '#/components/schemas/' } + '400': { description: Validation failed, content: { application/json: { schema: { $ref: '#/components/schemas/ErrorResponse' } } } } + '401': { description: Unauthenticated, content: { application/json: { schema: { $ref: '#/components/schemas/ErrorResponse' } } } } + '403': { description: Missing permission, content: { application/json: { schema: { $ref: '#/components/schemas/ErrorResponse' } } } } +``` + +## Validating the contract in CI + +The `sdd-gate.yml` workflow runs, on PRs that touch a `api-contract.yaml`: + +```bash +npx @stoplight/spectral-cli lint .specify/features/**/api-contract.yaml +``` + +Spectral's default OpenAPI ruleset catches malformed specs, missing `operationId`s, and +undefined `$ref`s. Add a `.spectral.yaml` at the repo root to tune rules if needed. diff --git a/.specify/templates/feature-spec.md b/.specify/templates/feature-spec.md new file mode 100644 index 00000000..4eb61303 --- /dev/null +++ b/.specify/templates/feature-spec.md @@ -0,0 +1,89 @@ + + +# I want so "> + +## Context & Why + + + +Constitution principles this feature depends on: +- [§ ](../../constitution.md#) — + +Related: . + +## User Journey + + + +## Requirements + +> One requirement per line, each with a `REQ-NNN` id and one EARS pattern. Include the +> patterns the feature actually needs — do not force all five, but a mutating feature almost +> always needs at least one Event-driven and one Unwanted-behavior requirement. + +- **REQ-001** (Ubiquitous) — The `` shall ``. +- **REQ-002** (Event-driven) — When ``, the `` shall ``. +- **REQ-003** (State-driven) — While ``, the `` shall ``. +- **REQ-004** (Optional-feature) — Where ``, the `` shall ``. +- **REQ-005** (Unwanted-behavior) — If ``, then the `` shall ``. + +## Acceptance Criteria + +> One measurable criterion per REQ-NNN. Numbers, limits, status codes — never adjectives. + +- **REQ-001** — . +- **REQ-002** — . +- **REQ-003** — . +- **REQ-004** — . +- **REQ-005** — . + +## Out of Scope + +- +- <…> + +## API / Contract Stub + + + +## Data Model Changes + +`, and the rollback note. Write "none" if not applicable.> + +## Security Considerations + + + +## Open Questions + +> Each item is a BLOCKER until resolved. Empty this list before implementation starts. + +- [ ] — owner: +- [ ] — owner: + +## Traceability + +| REQ-ID | Task ID(s) | Test ID(s) | Status | +|---|---|---|---| +| REQ-001 | | | Planned | +| REQ-002 | | | Planned | + + + +## Persona Review Results + +| Persona | Status | Key Findings | Resolved | +|---|---|---|---| +| Requirements Engineer | PENDING | | | +| Developer | PENDING | | | +| Security | PENDING | | | +| DevOps | PENDING | | | +| UI/UX | PENDING | | | +| Architect | PENDING | | | diff --git a/.specify/templates/threat-model.md b/.specify/templates/threat-model.md new file mode 100644 index 00000000..db063555 --- /dev/null +++ b/.specify/templates/threat-model.md @@ -0,0 +1,51 @@ + + +# Threat Model — + +**Feature spec:** [./spec.md](./spec.md) +**Date:** +**Author:** + +## Data Flow Diagram (text) + +**Actors** +- + +**Trust boundaries** +- TB-1: Browser ⇄ Caddy (public internet ⇄ DMZ) +- TB-2: Caddy ⇄ Backend (`:8080`) (DMZ ⇄ app) +- TB-3: Backend ⇄ PostgreSQL / MinIO / sidecars (app ⇄ data plane) +- + +**Data flows** (source → [boundary] → sink : data) +- F-1: Browser → [TB-1,TB-2] → Backend : +- F-2: Backend → [TB-3] → MinIO : +- <…> + +## STRIDE + +| Threat Category | Asset / Flow | Threat Description | Mitigation | Likelihood × Impact | Status | +|---|---|---|---|---|---| +| **S**poofing | | | | Low × High | | +| **T**ampering | | | | Med × High | | +| **R**epudiation | | | | Low × Med | | +| **I**nformation disclosure | | | | Med × High | | +| **D**enial of service | | | | Med × Med | | +| **E**levation of privilege | | | | Low × High | | + +## ASTRIDE (only if the feature invokes an AI agent / tool — OCR, NLP, LLM) + +| Threat | Asset / Flow | Threat Description | Mitigation | Likelihood × Impact | Status | +|---|---|---|---|---|---| +| Prompt Injection | | | | | | +| Context Poisoning | | | | | | +| Unsafe Tool Invocation | | | | | | +| Reasoning Subversion | | | | | | + +## Residual Risk + + -- 2.49.1 From 2dd7e33aadf3095c74ea804d365b27a859b07744 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 13 Jun 2026 11:56:37 +0200 Subject: [PATCH 03/10] feat(sdd): add Gitea issue templates and SDD CI gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds .gitea/ISSUE_TEMPLATE/{feature,bug}.md (the feature template mirrors the EARS feature-spec) and .gitea/workflows/sdd-gate.yml — spec-lint, contract validation (Spectral), traceability check (all non-blocking during adoption), and a constitution-impact PR comment. Co-Authored-By: Claude Opus 4.8 --- .gitea/ISSUE_TEMPLATE/bug.md | 40 ++++++ .gitea/ISSUE_TEMPLATE/feature.md | 82 ++++++++++++ .gitea/workflows/sdd-gate.yml | 206 +++++++++++++++++++++++++++++++ 3 files changed, 328 insertions(+) create mode 100644 .gitea/ISSUE_TEMPLATE/bug.md create mode 100644 .gitea/ISSUE_TEMPLATE/feature.md create mode 100644 .gitea/workflows/sdd-gate.yml diff --git a/.gitea/ISSUE_TEMPLATE/bug.md b/.gitea/ISSUE_TEMPLATE/bug.md new file mode 100644 index 00000000..2805094d --- /dev/null +++ b/.gitea/ISSUE_TEMPLATE/bug.md @@ -0,0 +1,40 @@ +--- +name: "Bug" +about: "Something is broken. Describe user-facing impact, not the technical cause." +title: " when " +labels: + - bug +assignees: [] +--- + + + +## What happens + + + +## Expected + + + +## Steps to reproduce + +1. +2. +3. + +## Originating requirement (if known) + + + +## Environment + + + +## Notes + + diff --git a/.gitea/ISSUE_TEMPLATE/feature.md b/.gitea/ISSUE_TEMPLATE/feature.md new file mode 100644 index 00000000..50cd8b5e --- /dev/null +++ b/.gitea/ISSUE_TEMPLATE/feature.md @@ -0,0 +1,82 @@ +--- +name: "Feature (SDD spec)" +about: "Spec-driven feature request. Fill in EARS requirements before implementation starts." +title: "As a I want so " +labels: + - spec-required + - needs-review +assignees: [] +--- + + + +## Context & Why + + + +## User Journey + + + +## Requirements + + + +- **REQ-001** (Ubiquitous) — The `` shall ``. +- **REQ-002** (Event-driven) — When ``, the `` shall ``. +- **REQ-003** (State-driven) — While ``, the `` shall ``. +- **REQ-004** (Optional-feature) — Where ``, the `` shall ``. +- **REQ-005** (Unwanted-behavior) — If ``, then the `` shall ``. + +## Acceptance Criteria + + + +- **REQ-001** — . +- **REQ-002** — . + +## Out of Scope + +- + +## API / Contract Stub + +/api-contract.yaml. Name new paths/methods/status codes and the @RequirePermission on each mutating endpoint.> + +## Data Model Changes + + (verify on disk) + rollback note. "none" if not applicable.> + +## Security Considerations + + + +## Open Questions + + + +- [ ] — owner: + +## Traceability + +| REQ-ID | Task ID(s) | Test ID(s) | Status | +|---|---|---|---| +| REQ-001 | | | Planned | + + + +## Persona Review Results + +| Persona | Status | Key Findings | Resolved | +|---|---|---|---| +| Requirements Engineer | PENDING | | | +| Developer | PENDING | | | +| Security | PENDING | | | +| DevOps | PENDING | | | +| UI/UX | PENDING | | | +| Architect | PENDING | | | diff --git a/.gitea/workflows/sdd-gate.yml b/.gitea/workflows/sdd-gate.yml new file mode 100644 index 00000000..9b0483cc --- /dev/null +++ b/.gitea/workflows/sdd-gate.yml @@ -0,0 +1,206 @@ +name: SDD Gate + +# Spec-Driven Development quality gate. Runs on PRs and validates the SDD artifacts that +# the PR touches. The first three jobs are NON-BLOCKING for now (continue-on-error) so the +# team can adopt the workflow without CI immediately failing. +# +# TODO: flip spec-lint, contract-validate, and traceability-check to BLOCKING +# (remove `continue-on-error: true`) once SDD adoption has settled — target: after the +# first 5 real features have shipped through the .specify/ workflow. Tracked in ADR-041. + +on: + pull_request: + +jobs: + # ─── Spec structure lint ────────────────────────────────────────────────────── + # Every modified .specify/features/*/spec.md must contain the required SDD sections + # and at least one REQ-NNN requirement. Pure grep — no external tooling. + spec-lint: + name: Spec Lint + runs-on: ubuntu-latest + continue-on-error: true # TODO: remove to make blocking (see header) + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Lint changed spec.md files + shell: bash + run: | + set -uo pipefail + base="origin/${{ github.event.pull_request.base.ref }}" + git fetch --no-tags --depth=1 origin "${{ github.event.pull_request.base.ref }}" || true + + required=( + "## Context & Why" + "## Requirements" + "## Acceptance Criteria" + "## Out of Scope" + "## Security Considerations" + "## Traceability" + ) + + # Self-test: a fixture with all sections + a REQ id must pass the section check. + fixture="$(mktemp)" + { for s in "${required[@]}"; do echo "$s"; done; echo "- **REQ-001** (Ubiquitous) — The x shall y."; } > "$fixture" + for s in "${required[@]}"; do + grep -qF "$s" "$fixture" || { echo "FAIL: spec-lint self-test missing '$s'"; exit 1; } + done + + changed="$(git diff --name-only "$base"...HEAD -- '.specify/features/*/spec.md' || true)" + if [ -z "$changed" ]; then + echo "No spec.md files changed — nothing to lint." + exit 0 + fi + + rc=0 + for f in $changed; do + [ -f "$f" ] || continue # deleted file + echo "── linting $f" + for s in "${required[@]}"; do + if ! grep -qF "$s" "$f"; then + echo "::error file=$f::missing required section '$s'" + rc=1 + fi + done + if ! grep -qE 'REQ-[0-9]{3}' "$f"; then + echo "::error file=$f::no REQ-NNN requirement found" + rc=1 + fi + done + exit $rc + + # ─── Contract validation ────────────────────────────────────────────────────── + # Validate any modified api-contract.yaml with Spectral (OpenAPI 3.1). REST stack — + # no GraphQL. Skips cleanly when no contract changed. + contract-validate: + name: Contract Validate + runs-on: ubuntu-latest + continue-on-error: true # TODO: remove to make blocking (see header) + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - uses: actions/setup-node@v4 + with: + node-version: '24' + + - name: Lint changed OpenAPI contracts + shell: bash + run: | + set -uo pipefail + base="origin/${{ github.event.pull_request.base.ref }}" + git fetch --no-tags --depth=1 origin "${{ github.event.pull_request.base.ref }}" || true + changed="$(git diff --name-only "$base"...HEAD -- '.specify/features/*/api-contract.yaml' || true)" + if [ -z "$changed" ]; then + echo "No api-contract.yaml changed — nothing to validate." + exit 0 + fi + rc=0 + for f in $changed; do + [ -f "$f" ] || continue + echo "── spectral lint $f" + npx --yes @stoplight/spectral-cli@6 lint "$f" || rc=1 + done + exit $rc + + # ─── Traceability check ─────────────────────────────────────────────────────── + # Warn (non-blocking) when a changed spec.md references a REQ-NNN that is absent from + # .specify/rtm.md. Keeps the matrix honest without hard-failing during adoption. + traceability-check: + name: Traceability Check + runs-on: ubuntu-latest + continue-on-error: true # TODO: remove to make blocking (see header) + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Cross-check spec REQ-IDs against rtm.md + shell: bash + run: | + set -uo pipefail + base="origin/${{ github.event.pull_request.base.ref }}" + git fetch --no-tags --depth=1 origin "${{ github.event.pull_request.base.ref }}" || true + changed="$(git diff --name-only "$base"...HEAD -- '.specify/features/*/spec.md' || true)" + [ -z "$changed" ] && { echo "No spec.md changed."; exit 0; } + test -f .specify/rtm.md || { echo "::warning::.specify/rtm.md missing"; exit 0; } + missing=0 + for f in $changed; do + [ -f "$f" ] || continue + for req in $(grep -oE 'REQ-[0-9]{3}' "$f" | sort -u); do + if ! grep -qF "$req" .specify/rtm.md; then + echo "::warning file=$f::$req is not present in .specify/rtm.md" + missing=$((missing+1)) + fi + done + done + echo "$missing REQ-ID(s) missing from rtm.md (warning only)." + + # ─── Constitution change impact ─────────────────────────────────────────────── + # When .specify/constitution.md is modified, list every file that references it (and so + # may need a Sync Impact update) and post it as a PR comment. Best-effort: if no token is + # available the list is only echoed to the log. This job is informational, never blocking. + constitution-diff: + name: Constitution Impact + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: List files referencing the constitution + id: impact + shell: bash + run: | + set -uo pipefail + base="origin/${{ github.event.pull_request.base.ref }}" + git fetch --no-tags --depth=1 origin "${{ github.event.pull_request.base.ref }}" || true + if ! git diff --name-only "$base"...HEAD -- '.specify/constitution.md' | grep -q .; then + echo "constitution.md not modified — skipping." + echo "changed=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + echo "changed=true" >> "$GITHUB_OUTPUT" + echo "Files referencing constitution.md (review for Sync Impact):" + # rg-free: grep recursively, excluding the file itself and VCS/build dirs. + grep -rIl --exclude-dir=.git --exclude-dir=node_modules --exclude-dir=target \ + -e 'constitution.md' -e 'constitution §' . \ + | grep -v '^\./.specify/constitution.md$' | sort > /tmp/refs.txt || true + cat /tmp/refs.txt + { + echo "body<> "$GITHUB_OUTPUT" + + - name: Post PR comment (best-effort) + if: steps.impact.outputs.changed == 'true' + shell: bash + env: + # Gitea Actions exposes the repo token as GITHUB_TOKEN; the API is Gitea-compatible. + TOKEN: ${{ secrets.GITHUB_TOKEN }} + SERVER: ${{ github.server_url }} + REPO: ${{ github.repository }} + PR: ${{ github.event.pull_request.number }} + BODY: ${{ steps.impact.outputs.body }} + run: | + set -uo pipefail + if [ -z "${TOKEN:-}" ]; then + echo "No token available — printing impact list to log only:" + echo "$BODY" + exit 0 + fi + payload="$(jq -n --arg b "$BODY" '{body:$b}')" + curl -sS -X POST \ + -H "Authorization: token ${TOKEN}" \ + -H "Content-Type: application/json" \ + "${SERVER}/api/v1/repos/${REPO}/issues/${PR}/comments" \ + -d "$payload" >/dev/null \ + && echo "Posted Sync Impact comment to PR #${PR}." \ + || { echo "Comment POST failed (non-fatal); impact list:"; echo "$BODY"; } -- 2.49.1 From acda4814723a14b9bb9a1bf3feefbd7560e5e6a0 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 13 Jun 2026 11:56:43 +0200 Subject: [PATCH 04/10] docs(sdd): add SDD onboarding guide and cross-reference from governance docs Adds SPEC_DRIVEN_DEVELOPMENT.md (8-step workflow, before/after issue, persona review example, agent-prompt example, maintenance rules, cheatsheet) and points CLAUDE.md, COLLABORATING.md, and CONTRIBUTING.md at the new .specify/ workflow without altering the existing cycle. Co-Authored-By: Claude Opus 4.8 --- CLAUDE.md | 4 + COLLABORATING.md | 8 ++ CONTRIBUTING.md | 1 + SPEC_DRIVEN_DEVELOPMENT.md | 157 +++++++++++++++++++++++++++++++++++++ 4 files changed, 170 insertions(+) create mode 100644 SPEC_DRIVEN_DEVELOPMENT.md diff --git a/CLAUDE.md b/CLAUDE.md index 810524b7..1181c41c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -16,6 +16,10 @@ See [COLLABORATING.md](./COLLABORATING.md) for the full rules: issue tracking wo See [CODESTYLE.md](./CODESTYLE.md) for coding standards: Clean Code, DRY/KISS trade-offs (KISS wins), and SOLID principles applied to this stack. +## Spec-Driven Development + +This project uses Spec-Driven Development. **Before implementing a feature, read [`.specify/AGENTS.md`](./.specify/AGENTS.md)** (the short, machine-readable agent rules) and obey the [`.specify/constitution.md`](./.specify/constitution.md) it references. A feature's contract is its `.specify/features//spec.md` (EARS `REQ-NNN` requirements) plus any `api-contract.yaml`. Full workflow: [SPEC_DRIVEN_DEVELOPMENT.md](./SPEC_DRIVEN_DEVELOPMENT.md); worked example: [`.specify/features/_example/`](./.specify/features/_example/). The LLM reminders below restate constitution rules — the constitution and AGENTS.md are authoritative if they ever diverge. + --- ## Stack diff --git a/COLLABORATING.md b/COLLABORATING.md index 532d80a8..c8e96198 100644 --- a/COLLABORATING.md +++ b/COLLABORATING.md @@ -8,6 +8,14 @@ Evaluate all suggestions on their technical merits. No sycophancy — if somethi ## Core Workflow: Research → Plan → Implement → Validate +> **Spec-Driven Development.** Feature work is front-ended by an SDD spec: EARS-formatted +> `REQ-NNN` requirements, persona spec-review checklists, and the project constitution. The +> sequence below is unchanged — SDD formalises its *inputs* (the issue body becomes a +> structured spec; the User Journey + E2E Scenarios below feed it). See +> [SPEC_DRIVEN_DEVELOPMENT.md](./SPEC_DRIVEN_DEVELOPMENT.md) and +> [`.specify/`](./.specify/) ([constitution](./.specify/constitution.md), +> [AGENTS.md](./.specify/AGENTS.md)). + Every non-trivial feature or bug fix follows this sequence: 1. **Research** — Read the relevant code. Understand existing patterns before touching anything. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7b7ba8a0..472bea43 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,6 +1,7 @@ # Contributing to Familienarchiv For the full collaboration rules (issue workflow, PR process, Red/Green TDD, commit conventions) see [COLLABORATING.md](./COLLABORATING.md). +For the Spec-Driven Development workflow (EARS specs, persona review, the constitution, and `.specify/`) see [SPEC_DRIVEN_DEVELOPMENT.md](./SPEC_DRIVEN_DEVELOPMENT.md). For coding style see [CODESTYLE.md](./CODESTYLE.md). For the system architecture see [docs/ARCHITECTURE.md](./docs/ARCHITECTURE.md) (introduced in DOC-2; until that PR merges, see [docs/architecture/c4-diagrams.md](./docs/architecture/c4-diagrams.md)). For domain terminology see [docs/GLOSSARY.md](./docs/GLOSSARY.md). diff --git a/SPEC_DRIVEN_DEVELOPMENT.md b/SPEC_DRIVEN_DEVELOPMENT.md new file mode 100644 index 00000000..1440bb52 --- /dev/null +++ b/SPEC_DRIVEN_DEVELOPMENT.md @@ -0,0 +1,157 @@ +# Spec-Driven Development (SDD) + +How we turn a feature idea into merged, traceable code in this repo. SDD layers a uniform, +machine-readable front-end onto the workflow we already run (Gitea issues → branch/PR → +multi-persona review → red/green TDD). It does not replace any of that — see +[ADR-041](./docs/adr/041-sdd-adoption.md) for the why. + +- **The rules** live in [`.specify/constitution.md`](./.specify/constitution.md) (humans) and + [`.specify/AGENTS.md`](./.specify/AGENTS.md) (AI agents, every invocation). +- **The templates** live in [`.specify/templates/`](./.specify/templates/). +- **The worked example** is [`.specify/features/_example/`](./.specify/features/_example/) — read it first. + +--- + +## 1. The workflow in 8 steps + +| # | Step | Who | Artifacts created / touched | +|---|---|---|---| +| 1 | **Idea → Gitea issue** using the Feature template | author | Gitea issue (labels `spec-required`, `needs-review`) from `.gitea/ISSUE_TEMPLATE/feature.md` | +| 2 | **Write the spec** — Context, User Journey, EARS `REQ-NNN` requirements, measurable acceptance criteria, Out of Scope | author | issue body **and** `.specify/features//spec.md` | +| 3 | **Add design artifacts** as needed | author | `design.md`; `api-contract.yaml` (any new endpoint); `threat-model.md` (uploads / new mutating endpoint / AI tool); feature-local `adr-NNN-*.md` or a `docs/adr/` entry for project-wide decisions | +| 4 | **Persona spec review** — the six checklists gate the spec | RE, Developer, Security, DevOps, UI/UX, Architect | `checklist-results.md` + the `## Persona Review Results` table; findings folded into the spec | +| 5 | **Resolve Open Questions & blocking FAILs** — spec does not proceed while any remain | author | spec updated; `Open Questions` emptied | +| 6 | **Decompose into tasks** in red/green order; seed the RTM | author | `tasks.md`; rows added to [`.specify/rtm.md`](./.specify/rtm.md) (`Status: Planned`) | +| 7 | **Implement** in a worktree, TDD per task (failing test → green → refactor → commit); agent reads `AGENTS.md` + `spec.md` + `api-contract.yaml` | implementer (often an AI agent) | code + tests; `npm run generate:api` after backend changes; RTM `Status` → `Done` | +| 8 | **PR → multi-persona PR review → merge**; archive the feature | reviewers | PR (`Closes #n`); on merge, move the feature dir under `.specify/features/_archive//` (or tag it shipped) | + +The personas at step 4 review the **spec**; the same personas at step 8 (via the existing +`review-pr` / `deliver-issue` skills) review the **code**. Step 4 catches at spec time what +used to surface only at step 8. + +## 2. How a Gitea issue becomes a spec + +**Before (free-form issue):** + +> **Title:** Add profile pictures +> Users should be able to upload a picture for their profile. Make sure it's not too big and +> only admins can remove other people's. Show initials if there's no picture. + +Ambiguous: how big? which formats? what status code on rejection? what about unauthenticated +callers? No identifiers to trace, no measurable criteria. + +**After (SDD-structured issue — excerpt):** + +> **Title:** As a user I want to upload a profile picture so other family members recognise me +> +> **## Requirements** +> - **REQ-002** (Event-driven) — When an authenticated user sends `POST /api/users/me/avatar` +> with a valid image, the user service shall store it and return a profile view with a +> non-null `avatarUrl`. +> - **REQ-008** (Unwanted-behavior) — If the uploaded file exceeds 2 MB, then the user service +> shall return `400 ErrorCode.AVATAR_TOO_LARGE` and store nothing. +> - **REQ-009** (Unwanted-behavior) — If a caller without `Permission.ADMIN_USER` targets +> another user's avatar, then the system shall return `403 ErrorCode.FORBIDDEN`. +> +> **## Acceptance Criteria** +> - **REQ-008** — a 2.1 MB PNG returns `400 AVATAR_TOO_LARGE`; bucket object count unchanged. + +Every behavior is now a uniquely-identified, testable, EARS-formed requirement with a +measurable acceptance criterion. See the full version in +[`.specify/features/_example/spec.md`](./.specify/features/_example/spec.md). + +## 3. How to run a persona review + +Each persona reads the spec, walks its checklist in `.specify/personas/.md`, and +posts a Gitea comment (or fills `checklist-results.md`) with **PASS / FAIL / QUESTION** per +item and a verdict. A `FAIL` from Security or Architect is a hard block. Concrete example: + +> ### Security — Spec Review +> +> | # | Item | Status | Note | +> |---|---|---|---| +> | 1 | All mutating endpoints have authn + authz `If` clauses | PASS | REQ-006 (401), REQ-009 (403) | +> | 3 | Audit fields server-set, forbidden in body | **FAIL** | `avatarObjectKey` is bound from the request body → mass-assignment (CWE-639). Make it server-set in `UserService`. | +> | 6 | Upload type allow-list + size | PASS | REQ-007 / REQ-008 | +> | 9 | threat-model.md present & STRIDE-complete | **QUESTION** | Is the avatar URL public or proxied? If public S3, that's information disclosure. | +> +> **Verdict: CHANGES REQUESTED** — blocking FAIL: #3. Resolve #9 in the threat model. + +The author folds the fix into the spec (here: server-set key + authenticated proxy URL), +empties the finding, and the persona re-reviews until `APPROVE`. This mirrors the existing +`review-issue` skill — the persona checklists just make the spec pass/fail explicit. + +## 4. How the AI agent uses the spec + +Once the spec is `APPROVE`d and tasks are seeded, the implementer points the agent at the +artifacts. Example prompt: + +> Implement `.specify/features/profile-picture-upload/`. Read `.specify/AGENTS.md` and obey +> the constitution it references. The contract is `spec.md` (REQ-001…REQ-009) and +> `api-contract.yaml`. Work through `tasks.md` in order, red/green TDD — write the failing +> test named in each task first, confirm it fails, then make it pass. After backend model +> changes run `npm run generate:api`. Each REQ has a test in the Traceability table; do not +> mark a task done until its test is green. Update `.specify/rtm.md` Status as you go. + +The agent now has: the rules (`AGENTS.md` → constitution), the exact requirements with ids, +the API shape, and a test-first task list — so its output is bounded and verifiable. + +## 5. Maintenance rules + +- **Constitution** ([`.specify/constitution.md`](./.specify/constitution.md)) — change it only + when a project-wide rule genuinely changes. Bump the semantic version (MAJOR = rule + removed/weakened, MINOR = rule added/tightened, PATCH = wording), run the §6 Sync Impact + review, and let the `constitution-diff` CI job list the files to reconcile. Record the bump + in ADR-041's revision log (or a superseding ADR for MAJOR). +- **AGENTS.md** — keep it under 200 lines. It cross-references the constitution; it must never + duplicate or contradict it. +- **ADRs** — project-wide decisions go in [`docs/adr/`](./docs/adr/) (next free `NNN`, verify + on disk). Immutable once `Accepted`; supersede, don't edit. Feature-local decisions stay + beside the feature spec. +- **Feature specs** — archive on merge: move `.specify/features//` to + `.specify/features/_archive//`. The spec stays as the record of what shipped. +- **RTM** ([`.specify/rtm.md`](./.specify/rtm.md)) — append rows when a spec is approved; + flip `Status` as tests go green; never delete a shipped requirement's row. CI warns on drift. +- **Personas** — update `.specify/personas/*.md` checklists when a recurring blind spot + appears; keep them aligned with the richer `.claude/personas/`. + +## 6. Quick-start cheatsheet + +**EARS patterns** (every requirement is one of these + a `REQ-NNN` id): + +| Pattern | Shape | +|---|---| +| Ubiquitous | `The shall .` | +| Event-driven | `When , the shall .` | +| State-driven | `While , the shall .` | +| Optional-feature | `Where , the shall .` | +| Unwanted-behavior | `If , then the shall .` | + +**File locations:** + +| What | Where | +|---|---| +| Non-negotiable rules | `.specify/constitution.md` | +| Agent rules (read every time) | `.specify/AGENTS.md` | +| Templates | `.specify/templates/{feature-spec,adr,threat-model,api-contract-stub}.md` | +| Persona checklists | `.specify/personas/*.md` | +| In-flight feature | `.specify/features//{spec,design,tasks,checklist-results}.md` + `api-contract.yaml` + `threat-model.md` | +| Worked example | `.specify/features/_example/` | +| Traceability matrix | `.specify/rtm.md` | +| ADR archive | `docs/adr/NNN-*.md` | +| Issue templates | `.gitea/ISSUE_TEMPLATE/{feature,bug}.md` | +| CI gate | `.gitea/workflows/sdd-gate.yml` | + +**Before you mark a feature done:** every `REQ-NNN` has a green test, the RTM Status is +`Done`, all six personas APPROVE, `npm run lint` and the targeted tests pass, and +`npm run generate:api` has been run if the backend model changed. + +**Commands:** + +```bash +# validate a contract locally (same as CI) +npx @stoplight/spectral-cli lint .specify/features//api-contract.yaml + +# regenerate the TS client after a backend model/endpoint change +cd frontend && npm run generate:api # backend must run with --spring.profiles.active=dev +``` -- 2.49.1 From f48a0d6dff667b80b3a46f17047d444e0aa05b5a Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 13 Jun 2026 11:56:49 +0200 Subject: [PATCH 05/10] feat(sdd): adopt review-issue, review-pr, implement skills to the SDD workflow review-issue becomes the SDD spec-review gate (adds the requirements engineer, pairs each .claude persona identity with its .specify checklist, EARS/REQ-NNN aware). review-pr verifies the diff against the constitution and the spec's REQ-NNN traceability. implement reads the spec artifacts, plans from tasks.md, ties tasks to REQ-NNN, and keeps the RTM and generated API types in sync. Co-Authored-By: Claude Opus 4.8 --- .claude/skills/implement/SKILL.md | 73 ++++++++++++++----- .claude/skills/review-issue/SKILL.md | 104 +++++++++++++++++---------- .claude/skills/review-pr/SKILL.md | 102 +++++++++++++++----------- 3 files changed, 185 insertions(+), 94 deletions(-) diff --git a/.claude/skills/implement/SKILL.md b/.claude/skills/implement/SKILL.md index d235d100..a7b76109 100644 --- a/.claude/skills/implement/SKILL.md +++ b/.claude/skills/implement/SKILL.md @@ -3,10 +3,17 @@ name: implement description: Felix Brandt reads a Gitea issue or Pull Request, clarifies ambiguities with the user, presents an implementation plan for approval, then works autonomously using red/green TDD until every task is done and committed. --- -# Implement — Felix Brandt's Issue/PR-Driven TDD Workflow +# Implement — Felix Brandt's Spec-Driven TDD Workflow You are Felix Brandt. Read your full persona from `.claude/personas/developer.md` before doing anything else. +Then load the SDD ground truth you must obey throughout: +- [`.specify/AGENTS.md`](../../../.specify/AGENTS.md) — stack, executable constraints, workflow rules, do-not-touch list +- [`.specify/constitution.md`](../../../.specify/constitution.md) — the non-negotiable rules AGENTS.md references + +The feature's `spec.md` (its `REQ-NNN` requirements) is the contract. Implement exactly what +the requirements say — no more, no less. + ## Argument The user provides a Gitea issue **or** pull request URL, e.g.: @@ -47,9 +54,20 @@ Mark each concern with its source: reviewer name + comment excerpt. Also read: - `CLAUDE.md` for project conventions +- **The feature's SDD artifacts**, if a spec exists for this issue: `.specify/features//spec.md` + (the `REQ-NNN` requirements + acceptance criteria), `tasks.md` (the red/green task list), + `api-contract.yaml` (the API shape), and `threat-model.md` (security obligations). If the + issue is a well-formed SDD spec but no `.specify/features//` directory exists yet, + create one from [the templates](../../../.specify/templates/) and mirror the spec into it. +- [`.specify/rtm.md`](../../../.specify/rtm.md) — note each `REQ-NNN`'s current Status - Any relevant existing source files mentioned in the issue/comments - The current branch state (`git status`, `git log --oneline -10`) +> **If the issue is NOT a well-formed SDD spec** (free-prose, no `REQ-NNN`, missing sections), +> stop before Phase 2 and tell the user: it should go through `/review-issue` (the SDD +> spec-review gate) first. Offer to help restructure it into a spec rather than implementing +> against an ambiguous issue. + Do not start Phase 2 until you have read everything. --- @@ -58,10 +76,12 @@ Do not start Phase 2 until you have read everything. ### Issue mode -After reading, identify every point that is genuinely ambiguous or underspecified — things you cannot safely decide unilaterally: -- Scope questions (is X in or out of this issue?) -- Design decisions with multiple valid approaches where the choice affects architecture -- Missing acceptance criteria (how do we know when this is done?) +First, check the spec's `## Open Questions` — **any unresolved item there is a blocker** and +must be answered before implementation (SDD step 5). Then identify any further point that is +genuinely ambiguous or underspecified — things you cannot safely decide unilaterally: +- Scope questions (is X in or out? — check `## Out of Scope` first) +- A `REQ-NNN` that is not testable as written, or has no measurable acceptance criterion +- Design decisions with multiple valid approaches where the choice affects architecture (if it's an irreversible choice, it may need an ADR — flag it) - Conflicting statements between the issue body and the comments - Dependencies on external things (backend changes needed? migration required?) @@ -81,12 +101,15 @@ Wait for the user to answer before continuing. ## Phase 3 — Implementation Plan -Once clarifications are resolved, present a numbered implementation plan as a task list. Each item must be: +Once clarifications are resolved, present a numbered implementation plan as a task list. **If +the spec has a `tasks.md`, the plan IS that task list** — confirm it, refine it, and surface +it for approval rather than inventing a parallel one. Each item must be: - A single atomic unit of work (one behavior, one file change, one migration) - Written as a sentence that implies the test name: "Tag detail page returns 404 when tag does not exist" -- Ordered so each item builds on the previous ones +- Ordered so each item builds on the previous ones (red/green order — a failing test precedes its implementation) - Prefixed with the layer: `[backend]`, `[frontend]`, `[migration]`, `[test]`, `[refactor]` +- **In issue/SDD mode, tagged with the `REQ-NNN` it satisfies** so every requirement is covered and nothing extra is built. Flag any requirement with no task (gap) and any task with no requirement (scope creep). **In PR mode**, each task must reference the reviewer concern it addresses, e.g.: ``` @@ -97,10 +120,10 @@ Format: ``` ## Implementation Plan -1. [backend] PersonController returns 404 when person id does not exist -2. [migration] Add index on documents.sender_id for performance -3. [frontend] PersonCard renders full name from firstName + lastName props -4. [frontend] PersonCard shows placeholder when both names are null +1. [backend] PersonController returns 404 when person id does not exist — REQ-006 +2. [migration] V add index on documents.sender_id (verify next free number on disk) — REQ-002 +3. [frontend] PersonCard renders full name from firstName + lastName props — REQ-004 +4. [frontend] PersonCard shows placeholder when both names are null — REQ-004 ... ``` @@ -145,12 +168,22 @@ Check the current branch. 2. Apply any needed clean-up — no new behavior 3. Run the full suite again to confirm still green +**Sync (SDD):** +1. If this task changed a backend model or endpoint, run `cd frontend && npm run generate:api` + (backend must be running with `--spring.profiles.active=dev`) and stage the regenerated types. +2. If this task added a new `ErrorCode`, confirm all four sites are updated (`ErrorCode.java`, + `frontend/src/lib/shared/errors.ts`, `getErrorMessage()`, `messages/{de,en,es}.json`). +3. Flip the task's `REQ-NNN` Status in [`.specify/rtm.md`](../../../.specify/rtm.md) and in the + spec's Traceability table to `Done`, filling in the implementation file(s) and test name. + **Commit:** -Commit atomically after each task using the project's commit conventions: +Commit atomically after each task using the project's commit conventions, referencing the +issue (`Refs #n` / `Closes #n`) on the last line: ``` feat(scope): short imperative description -Co-Authored-By: Claude Sonnet 4.6 +Refs # +Co-Authored-By: ``` Move to the next task immediately. @@ -164,8 +197,10 @@ Move to the next task immediately. ### Rules during autonomous implementation +- Obey the constitution and AGENTS.md at all times — especially the §4 Do-Not-Touch list (never edit generated files, shipped migrations, or an Accepted ADR; never bump the artifact action past v3; never weaken a CI guard). - Never skip the red step — if you cannot write a failing test for a task, stop and explain why to the user before writing any implementation code -- Never add behavior beyond what the current task requires +- Never add behavior beyond what the current task requires — and never add behavior with no backing `REQ-NNN`. If implementation reveals a genuinely missing requirement, stop and raise it (it becomes a new REQ in the spec), don't silently scope-creep. +- An irreversible decision discovered mid-implementation (new dependency, new domain, data-model shape) needs an ADR in `docs/adr/` (next free number, verified on disk) before you bake it in — stop and flag it. - Never bundle two tasks into one commit - If a test that was passing starts failing during a later task, fix it before continuing — do not leave broken tests - If you hit a genuine blocker (missing API, infrastructure not available, etc.) that prevents completing a task, stop and report it to the user rather than working around it silently @@ -178,10 +213,16 @@ After all tasks are done: 1. Run the full test suite one final time and confirm all green 2. Run `npm run check` (frontend) and `./mvnw clean package -DskipTests` (backend) to confirm no type or build errors +3. **SDD traceability gate:** confirm every `REQ-NNN` in the spec has a green test and is marked + `Done` in [`.specify/rtm.md`](../../../.specify/rtm.md). Any requirement without a passing + test means the feature is not done — go back and finish it. Confirm `generate:api` was run + if any backend model/endpoint changed. ### Issue mode -3. Post a completion comment on the Gitea issue summarising what was implemented, listing all commits made -4. Report back to the user: every task ✅, any skipped/deferred tasks (with reason), the branch name, next suggested action (open PR, run `/review-pr`, etc.) +4. Post a completion comment on the Gitea issue summarising what was implemented, mapping each + `REQ-NNN` to its commit and test, and listing all commits made +5. Report back to the user: every task ✅, the REQ→test coverage, any skipped/deferred tasks + (with reason), the branch name, next suggested action (open PR, run `/review-pr`, etc.) ### PR mode 3. Push the updated branch diff --git a/.claude/skills/review-issue/SKILL.md b/.claude/skills/review-issue/SKILL.md index 75a538ad..5d2a27fa 100644 --- a/.claude/skills/review-issue/SKILL.md +++ b/.claude/skills/review-issue/SKILL.md @@ -1,13 +1,15 @@ --- name: review-issue -description: Multi-persona feature issue review. Each persona from .claude/personas/ reads the issue and posts constructive feedback as a separate Gitea comment. +description: Multi-persona SDD spec review of a Gitea feature issue. Each persona pairs its .claude/personas/ identity with its .specify/personas/ checklist, walks it PASS/FAIL/QUESTION against the EARS requirements, and posts findings as a separate Gitea comment before implementation starts. --- -# Multi-Persona Feature Issue Review +# Multi-Persona Spec Review (SDD) -You will perform a thorough multi-persona review of the given Gitea issue URL and post each persona's constructive feedback as a **separate comment** on the issue. - -Personas give **advisory input only** — no blocking, no verdicts. The goal is to surface blind spots, risks, and improvement ideas before implementation starts. +You will perform a thorough multi-persona **spec review** of the given Gitea feature issue and +post each persona's findings as a **separate comment** on the issue. This is the SDD +spec-review gate (step 4 of [SPEC_DRIVEN_DEVELOPMENT.md](../../../SPEC_DRIVEN_DEVELOPMENT.md)): +the goal is to catch ambiguity, missing requirements, and blind spots **before** any code is +written, while the cost of change is a sentence edit. ## Argument @@ -19,57 +21,83 @@ Parse it to extract: - `repo` — e.g. `familienarchiv` - `issue_number` — e.g. `161` -## Step 1 — Gather Issue Context +## Step 0 — Load the SDD ground truth + +Before reading the issue, read the rules every persona reviews against: +- [`.specify/constitution.md`](../../../.specify/constitution.md) — the non-negotiable rules +- [`.specify/AGENTS.md`](../../../.specify/AGENTS.md) — stack, constraints, workflow +- [`.specify/templates/feature-spec.md`](../../../.specify/templates/feature-spec.md) — the expected spec shape and the five EARS patterns +- The worked example [`.specify/features/_example/spec.md`](../../../.specify/features/_example/spec.md) — what "good" looks like + +## Step 1 — Gather issue context Use the Gitea MCP tools to collect: 1. The full issue (title, body, labels, milestone, assignees) via `issue_read` -2. All existing comments on the issue via `issue_read` — read them so personas don't repeat what's already been said +2. All existing comments — read them so personas don't repeat what's already been said Read everything before starting any review. -## Step 2 — Read Every Persona +## Step 2 — Read every persona (identity + checklist) -Read all six persona files from `.claude/personas/`: -- `developer.md` → Felix Brandt -- `architect.md` → architect persona -- `tester.md` → tester persona -- `security_expert.md` → security persona -- `ui_expert.md` → UI/UX persona -- `devops.md` → DevOps persona +Each persona is its **character identity** (`.claude/personas/`) **plus** its **SDD spec-review +checklist** (`.specify/personas/`). Adopt the voice from the former; gate the spec with the latter. -## Step 3 — Write Each Review +| Persona | Identity (`.claude/personas/`) | Checklist (`.specify/personas/`) | +|---|---|---| +| Requirements Engineer | `req_engineer.md` | `requirements-engineer.md` | +| Developer (Felix Brandt) | `developer.md` | `developer.md` | +| Security (Nora "NullX" Steiner) | `security_expert.md` | `security.md` | +| DevOps | `devops.md` | `devops.md` | +| UI/UX | `ui_expert.md` | `ui-ux.md` | +| Architect | `architect.md` | `architect.md` | -For each persona, fully adopt their identity, priorities, and thinking style as described in their persona file. Write feedback that: +The tester lens (acceptance-criteria quality, edge cases) is carried by the Requirements +Engineer checklist (testable, measurable criteria) — no separate tester comment at spec time. -- Is **constructive and forward-looking** — no blockers, no verdicts, no approval stamps -- Asks clarifying questions the persona would genuinely want answered before or during implementation -- Points out risks, edge cases, or gaps the persona sees from their domain -- Offers concrete suggestions or alternative approaches where relevant -- References the issue text specifically — don't write generic advice -- Stays focused on what the persona would actually care about (e.g. Felix asks about test strategy and naming; the architect asks about layer boundaries and coupling; the security expert asks about auth, input validation, and data exposure; the tester asks about acceptance criteria and edge cases; the UI expert asks about interaction patterns and accessibility; DevOps asks about deployment, config, and observability) +## Step 3 — Run each checklist against the spec -Format each comment in Markdown with a persona header, e.g.: +For each persona, walk **every item** in its `.specify/personas/` checklist and assign +**PASS / FAIL / QUESTION**, judged against the constitution and the issue text: + +- **EARS-aware:** verify each requirement uses one of the five EARS patterns and carries a + `REQ-NNN` id. The Requirements Engineer leads here; every persona flags missing + Unwanted-behavior (`If …`) clauses in their domain (Security especially — a mutating + endpoint with no `If` clause for unauthenticated/unauthorized access is an automatic FAIL). +- **If the issue is not yet an SDD spec** (free-prose, no `REQ-NNN`, missing sections), the + Requirements Engineer's primary finding is to restructure it using the feature-spec + template, and other personas review what they can while noting the gap. +- Reference the issue text specifically — quote the requirement or the missing section. No + generic advice. + +## Step 4 — Write and post each comment + +Each persona posts a **separate** comment via the Gitea MCP `issue_write` tool, in the format +its checklist's "Output format" section defines — a header, the checklist table, and a verdict: ``` -## 👨‍💻 Felix Brandt — Senior Fullstack Developer +### 🔐 Security — Spec Review -### Questions & Observations -... +| # | Item | Status | Note | +|---|------|--------|------| +| 1 | All mutating endpoints have authn + authz `If` clauses | FAIL | REQ-004 POST has no 401 clause (CWE-...) | +| 2 | ... | PASS | | -### Suggestions -... +**Verdict: CHANGES REQUESTED** — blocking FAIL: #1. Resolve before implementation. ``` -Keep each comment focused and scannable. Use bullet points. Avoid walls of text. +Post all six comments. If a persona's checklist is entirely PASS, still post the table and a +`Verdict: APPROVE` so the team knows the perspective was applied. Keep comments scannable. -## Step 4 — Post Comments +These verdicts are a **pre-implementation gate**, not a PR merge gate: a `FAIL` means the +issue/spec must be amended (per SDD step 5) before work starts. Fold the agreed fixes into +the issue description (the issue body is the source of truth), then re-run this review with +clean context rather than leaving a long comment thread. -Post each persona's feedback as a **separate comment** on the issue using the Gitea MCP `issue_write` tool. - -Post all six comments. If a persona genuinely has nothing to add (rare), write a short "No concerns from my angle" with one sentence explaining what they checked — so the team knows that perspective was considered. - -## Step 5 — Report Back +## Step 5 — Report back After all comments are posted, tell the user: -- Which personas posted feedback -- A brief summary of the most important cross-cutting themes (questions or risks that multiple personas flagged) +- Each persona's verdict (APPROVE / CHANGES REQUESTED) +- The consolidated list of blocking FAILs (these must be resolved before implementation) +- Cross-cutting themes multiple personas flagged +- Whether the issue is a well-formed SDD spec yet, or needs restructuring first +- A reminder to mirror the agreed `REQ-NNN` rows into [`.specify/rtm.md`](../../../.specify/rtm.md) diff --git a/.claude/skills/review-pr/SKILL.md b/.claude/skills/review-pr/SKILL.md index 9712de90..da715ec6 100644 --- a/.claude/skills/review-pr/SKILL.md +++ b/.claude/skills/review-pr/SKILL.md @@ -1,74 +1,96 @@ --- name: review-pr -description: Multi-persona PR review. Each persona from .claude/personas/ reviews the PR and posts their findings as a separate Gitea comment. +description: Multi-persona SDD code review of a Gitea PR. Each persona pairs its .claude/personas/ identity with its .specify/personas/ checklist, verifies the diff against the constitution and the feature spec's REQ-NNN (every requirement implemented and tested), and posts findings as a separate Gitea comment. --- -# Multi-Persona PR Review +# Multi-Persona PR Review (SDD) -You will perform a thorough multi-persona code review of the given PR URL and post each persona's findings as a **separate comment** on the PR. +You will perform a thorough multi-persona code review of the given PR and post each persona's +findings as a **separate comment**. Under SDD, the review verifies the diff against two +contracts: the project [constitution](../../../.specify/constitution.md) and the feature's +spec (`spec.md` — every `REQ-NNN` must be implemented **and** covered by a test). ## Argument The user provides a Gitea PR URL, e.g.: `http://heim-nas:3005/marcel/familienarchiv/pulls/160` -Parse it to extract: -- `owner` — e.g. `marcel` -- `repo` — e.g. `familienarchiv` -- `pull_number` — e.g. `160` +Parse it to extract `owner`, `repo`, and `pull_number`. -## Step 1 — Gather PR Context +## Step 0 — Load the SDD ground truth + +Read before reviewing: +- [`.specify/constitution.md`](../../../.specify/constitution.md) — rules the code must obey (esp. §4 Do-Not-Touch) +- [`.specify/AGENTS.md`](../../../.specify/AGENTS.md) — constraints +- The feature's spec, if the PR references one: `.specify/features//spec.md`, plus its + `api-contract.yaml`, `threat-model.md`, and `tasks.md`. Find the `REQ-NNN` ids it claims to + satisfy (from the PR description, `Closes #n`, or the spec's Traceability table). +- [`.specify/rtm.md`](../../../.specify/rtm.md) — the requirement→test→status matrix + +## Step 1 — Gather PR context Use the Gitea MCP tools to collect: -1. PR metadata (title, description, base branch, head branch) via `pull_request_read` -2. The list of changed files via `get_dir_contents` or the PR files endpoint -3. The full diff / file contents of every changed file — read each file at the head commit using `get_file_contents` +1. PR metadata (title, description, base/head branch) via `pull_request_read` +2. The list of changed files +3. The full content of every changed file at the head commit via `get_file_contents` -Read ALL changed files completely before starting any review. Do not skip files. +Read ALL changed files completely before starting. Do not skip files. -## Step 2 — Read Every Persona +## Step 2 — Read every persona (identity + checklist) -Read all six persona files from `.claude/personas/`: -- `developer.md` → Felix Brandt -- `architect.md` → architect persona -- `tester.md` → tester persona -- `security_expert.md` → security persona -- `ui_expert.md` → UI/UX persona -- `devops.md` → DevOps persona +Adopt each persona's voice from `.claude/personas/`; apply its review lens. For the SDD +personas, also re-read the matching `.specify/personas/` checklist — at PR time the same +checklist items are verified against the **code** rather than the spec. -## Step 3 — Write Each Review +| Persona | Identity (`.claude/personas/`) | Checklist (`.specify/personas/`) | PR-time focus | +|---|---|---|---| +| Requirements Engineer | `req_engineer.md` | `requirements-engineer.md` | Traceability: every `REQ-NNN` implemented; RTM updated | +| Developer (Felix Brandt) | `developer.md` | `developer.md` | Clean code, layering, generate:api run, ErrorCode four-site | +| Tester | `tester.md` | — (uses identity) | Test quality: each REQ has a real failing-first test; edge cases; levels right | +| Security (Nora "NullX") | `security_expert.md` | `security.md` | authn/authz, IDOR, mass-assignment, `{@html}`, secrets/PII | +| DevOps | `devops.md` | `devops.md` | migration rollback, env vars, CI guards intact, artifact pin | +| UI/UX | `ui_expert.md` | `ui-ux.md` | states, i18n, a11y, design tokens | +| Architect | `architect.md` | `architect.md` | boundaries, ADR present for irreversible choices, no superseded-ADR violation | -For each persona, fully adopt their identity, priorities, and review lens as described in their persona file. Write a review that: +## Step 3 — Write each review + +For each persona, write a review that: - Opens with a one-line verdict: **✅ Approved**, **⚠️ Approved with concerns**, or **🚫 Changes requested** -- Lists concrete findings with file paths and line references where relevant -- Distinguishes blockers (must fix) from suggestions (nice to have) -- Uses the persona's voice and priorities (e.g. Felix cares about TDD and clean code; the security expert checks for injection, auth, and data exposure; the architect checks layer boundaries and coupling) -- Stays focused — only comment on what the persona would actually care about - -Format each comment in Markdown with a persona header, e.g.: +- Lists concrete findings with file paths and line references; cite the constitution rule + (e.g. "violates §2.4 — `updatedBy` bound from request body") or the `REQ-NNN` at issue +- Distinguishes **blockers** (must fix) from **suggestions** (nice to have) +- **Requirements Engineer specifically** produces a traceability table — for each `REQ-NNN`: + is it implemented? is there a test? is `rtm.md` updated to `Done`? Any unimplemented or + untested REQ is a blocker. Any code behavior with no backing requirement is flagged + (scope creep — should it be a new REQ, or removed?). +- A constitution **Do-Not-Touch** violation (edited generated file, edited shipped migration, + edited an Accepted ADR, bumped the artifact action past v3, weakened a CI guard) is always + a blocker. ``` -## 👨‍💻 Felix Brandt — Senior Fullstack Developer +### 🔐 Security — PR Review **Verdict: ⚠️ Approved with concerns** ### Blockers -... +- `UserAvatarController.java:42` — REQ-009's 403 path has no test (constitution §2.8) ### Suggestions -... +- ... ``` -## Step 4 — Post Comments +## Step 4 — Post comments -Post each persona's review as a **separate comment** on the PR using the Gitea MCP `issue_write` tool (issues and PRs share the comment API in Gitea). +Post each persona's review as a **separate comment** via the Gitea MCP `issue_write` tool +(issues and PRs share the comment API). Post all personas; if one has nothing to flag, post a +brief "LGTM" naming what they checked. -Post all six comments. Do not skip any persona even if their domain has nothing to flag — in that case write a brief "LGTM" with a short explanation of what they checked. +## Step 5 — Report back -## Step 5 — Report Back - -After all comments are posted, summarize to the user: -- Which personas posted comments -- The overall verdict across all personas (worst-case wins: if any said "Changes requested", the overall is "Changes requested") -- A bullet list of the top blockers found (if any) +Summarize to the user: +- Each persona's verdict and the overall verdict (worst-case wins: any "Changes requested" → overall "Changes requested") +- The full list of blockers, grouped by persona +- **Traceability status:** which `REQ-NNN` are implemented+tested vs. missing, and whether + `rtm.md` is in sync +- Any constitution Do-Not-Touch violations (called out explicitly) -- 2.49.1 From 40c1bba113e86a87a665e5a2ee58cbe0a3529557 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 13 Jun 2026 12:14:32 +0200 Subject: [PATCH 06/10] refactor(sdd): make the feature spec issue-only (no committed spec.md) The Gitea issue body is the single source of truth for a spec; the only per-feature artifact in git is the RTM row (REQ-ID -> issue # -> test). Drops per-feature spec.md/tasks.md/checklist files from the workflow (the _example stays as a template/reference). Updates the guide, ADR-041, AGENTS.md, CLAUDE.md, templates, the RTM (adds an Issue column), the implement/review-pr skills, and replaces the file-spec CI jobs with an rtm-check. Co-Authored-By: Claude Opus 4.8 --- .claude/skills/implement/SKILL.md | 17 ++- .claude/skills/review-pr/SKILL.md | 7 +- .gitea/ISSUE_TEMPLATE/feature.md | 9 +- .gitea/workflows/sdd-gate.yml | 136 ++++++++---------------- .specify/AGENTS.md | 9 +- .specify/personas/developer.md | 4 +- .specify/rtm.md | 47 ++++---- .specify/templates/api-contract-stub.md | 22 ++-- .specify/templates/feature-spec.md | 20 ++-- .specify/templates/threat-model.md | 10 +- CLAUDE.md | 2 +- SPEC_DRIVEN_DEVELOPMENT.md | 77 ++++++++------ docs/adr/041-sdd-adoption.md | 20 ++-- 13 files changed, 178 insertions(+), 202 deletions(-) diff --git a/.claude/skills/implement/SKILL.md b/.claude/skills/implement/SKILL.md index a7b76109..47019d78 100644 --- a/.claude/skills/implement/SKILL.md +++ b/.claude/skills/implement/SKILL.md @@ -54,12 +54,11 @@ Mark each concern with its source: reviewer name + comment excerpt. Also read: - `CLAUDE.md` for project conventions -- **The feature's SDD artifacts**, if a spec exists for this issue: `.specify/features//spec.md` - (the `REQ-NNN` requirements + acceptance criteria), `tasks.md` (the red/green task list), - `api-contract.yaml` (the API shape), and `threat-model.md` (security obligations). If the - issue is a well-formed SDD spec but no `.specify/features//` directory exists yet, - create one from [the templates](../../../.specify/templates/) and mirror the spec into it. -- [`.specify/rtm.md`](../../../.specify/rtm.md) — note each `REQ-NNN`'s current Status +- **The issue body — it IS the spec** (issue-only; there is no committed `spec.md`). Extract its + `REQ-NNN` requirements, acceptance criteria, API stub, data-model delta, and any inline + STRIDE/threat notes. These are your contract. +- [`.specify/rtm.md`](../../../.specify/rtm.md) — note each `REQ-NNN`'s current Status (rows are + keyed by this issue number) - Any relevant existing source files mentioned in the issue/comments - The current branch state (`git status`, `git log --oneline -10`) @@ -101,9 +100,9 @@ Wait for the user to answer before continuing. ## Phase 3 — Implementation Plan -Once clarifications are resolved, present a numbered implementation plan as a task list. **If -the spec has a `tasks.md`, the plan IS that task list** — confirm it, refine it, and surface -it for approval rather than inventing a parallel one. Each item must be: +Once clarifications are resolved, present a numbered implementation plan as a task list, +**derived from the issue's `REQ-NNN` requirements** (one or more tasks per requirement, in +red/green order). Each item must be: - A single atomic unit of work (one behavior, one file change, one migration) - Written as a sentence that implies the test name: "Tag detail page returns 404 when tag does not exist" diff --git a/.claude/skills/review-pr/SKILL.md b/.claude/skills/review-pr/SKILL.md index da715ec6..1907c3e9 100644 --- a/.claude/skills/review-pr/SKILL.md +++ b/.claude/skills/review-pr/SKILL.md @@ -8,7 +8,7 @@ description: Multi-persona SDD code review of a Gitea PR. Each persona pairs its You will perform a thorough multi-persona code review of the given PR and post each persona's findings as a **separate comment**. Under SDD, the review verifies the diff against two contracts: the project [constitution](../../../.specify/constitution.md) and the feature's -spec (`spec.md` — every `REQ-NNN` must be implemented **and** covered by a test). +spec (the linked **Gitea issue body** — every `REQ-NNN` must be implemented **and** covered by a test). ## Argument @@ -22,9 +22,8 @@ Parse it to extract `owner`, `repo`, and `pull_number`. Read before reviewing: - [`.specify/constitution.md`](../../../.specify/constitution.md) — rules the code must obey (esp. §4 Do-Not-Touch) - [`.specify/AGENTS.md`](../../../.specify/AGENTS.md) — constraints -- The feature's spec, if the PR references one: `.specify/features//spec.md`, plus its - `api-contract.yaml`, `threat-model.md`, and `tasks.md`. Find the `REQ-NNN` ids it claims to - satisfy (from the PR description, `Closes #n`, or the spec's Traceability table). +- The feature's spec — the **Gitea issue** the PR closes (`Closes #n`). Read its body for the + `REQ-NNN` requirements, acceptance criteria, inline API stub, and any STRIDE/threat notes. - [`.specify/rtm.md`](../../../.specify/rtm.md) — the requirement→test→status matrix ## Step 1 — Gather PR context diff --git a/.gitea/ISSUE_TEMPLATE/feature.md b/.gitea/ISSUE_TEMPLATE/feature.md index 50cd8b5e..a9d56b09 100644 --- a/.gitea/ISSUE_TEMPLATE/feature.md +++ b/.gitea/ISSUE_TEMPLATE/feature.md @@ -9,10 +9,9 @@ assignees: [] --- ## Context & Why @@ -46,7 +45,7 @@ assignees: [] ## API / Contract Stub -/api-contract.yaml. Name new paths/methods/status codes and the @RequirePermission on each mutating endpoint.> + ## Data Model Changes diff --git a/.gitea/workflows/sdd-gate.yml b/.gitea/workflows/sdd-gate.yml index 9b0483cc..6763ddbc 100644 --- a/.gitea/workflows/sdd-gate.yml +++ b/.gitea/workflows/sdd-gate.yml @@ -1,78 +1,64 @@ name: SDD Gate -# Spec-Driven Development quality gate. Runs on PRs and validates the SDD artifacts that -# the PR touches. The first three jobs are NON-BLOCKING for now (continue-on-error) so the -# team can adopt the workflow without CI immediately failing. +# Spec-Driven Development quality gate. Runs on PRs. # -# TODO: flip spec-lint, contract-validate, and traceability-check to BLOCKING -# (remove `continue-on-error: true`) once SDD adoption has settled — target: after the -# first 5 real features have shipped through the .specify/ workflow. Tracked in ADR-041. +# This project is ISSUE-ONLY: a feature's spec lives in its Gitea issue body, not a committed +# spec.md (see ADR-041). So CI cannot lint the spec text itself — instead it validates the SDD +# artifacts that DO live in git: the RTM, any committed OpenAPI contract, and the constitution. +# +# The first two jobs are NON-BLOCKING for now (continue-on-error) so the team can adopt the +# workflow without CI immediately failing. +# +# TODO: flip rtm-check and contract-validate to BLOCKING (remove `continue-on-error: true`) +# once SDD adoption has settled — target: after the first 5 features have shipped through +# the workflow. Tracked in ADR-041. on: pull_request: jobs: - # ─── Spec structure lint ────────────────────────────────────────────────────── - # Every modified .specify/features/*/spec.md must contain the required SDD sections - # and at least one REQ-NNN requirement. Pure grep — no external tooling. - spec-lint: - name: Spec Lint + # ─── RTM check ──────────────────────────────────────────────────────────────── + # The Requirements Traceability Matrix is the one per-feature SDD artifact in git. Every + # data row must point at a Gitea issue (`#n`) and name at least one test. Warn otherwise. + # Pure awk — no external tooling. Columns: | REQ-ID | Summary | Issue | Feature | Impl | Test | Status | + rtm-check: + name: RTM Check runs-on: ubuntu-latest continue-on-error: true # TODO: remove to make blocking (see header) steps: - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - name: Lint changed spec.md files + - name: Validate .specify/rtm.md rows shell: bash run: | set -uo pipefail - base="origin/${{ github.event.pull_request.base.ref }}" - git fetch --no-tags --depth=1 origin "${{ github.event.pull_request.base.ref }}" || true + rtm=".specify/rtm.md" + test -f "$rtm" || { echo "::error::$rtm is missing"; exit 1; } - required=( - "## Context & Why" - "## Requirements" - "## Acceptance Criteria" - "## Out of Scope" - "## Security Considerations" - "## Traceability" - ) + # Self-test: a good row passes, a row with an empty Issue or Test is flagged. + check_row() { awk -F'|' '{ + issue=$4; test_col=$7; + gsub(/^[ \t]+|[ \t]+$/,"",issue); gsub(/^[ \t]+|[ \t]+$/,"",test_col); + if (issue !~ /#/ || test_col=="") exit 1; else exit 0 }'; } + echo '| REQ-001 | x | #42 | f | impl | SomeTest#works | Done |' | check_row \ + || { echo "FAIL: rtm-check self-test rejected a valid row"; exit 1; } + echo '| REQ-002 | x | | f | impl | | Planned |' | check_row \ + && { echo "FAIL: rtm-check self-test accepted an empty row"; exit 1; } - # Self-test: a fixture with all sections + a REQ id must pass the section check. - fixture="$(mktemp)" - { for s in "${required[@]}"; do echo "$s"; done; echo "- **REQ-001** (Ubiquitous) — The x shall y."; } > "$fixture" - for s in "${required[@]}"; do - grep -qF "$s" "$fixture" || { echo "FAIL: spec-lint self-test missing '$s'"; exit 1; } - done - - changed="$(git diff --name-only "$base"...HEAD -- '.specify/features/*/spec.md' || true)" - if [ -z "$changed" ]; then - echo "No spec.md files changed — nothing to lint." - exit 0 - fi - - rc=0 - for f in $changed; do - [ -f "$f" ] || continue # deleted file - echo "── linting $f" - for s in "${required[@]}"; do - if ! grep -qF "$s" "$f"; then - echo "::error file=$f::missing required section '$s'" - rc=1 - fi - done - if ! grep -qE 'REQ-[0-9]{3}' "$f"; then - echo "::error file=$f::no REQ-NNN requirement found" - rc=1 - fi - done - exit $rc + bad=0 + while IFS= read -r line; do + echo "$line" | check_row || { + req=$(echo "$line" | awk -F'|' '{gsub(/^[ \t]+|[ \t]+$/,"",$2); print $2}') + echo "::warning file=$rtm::row $req is missing an Issue (#n) or a Test" + bad=$((bad+1)) + } + done < <(grep -E '^\| REQ-[0-9]{3} ' "$rtm") + echo "$bad RTM row(s) incomplete (warning only)." # ─── Contract validation ────────────────────────────────────────────────────── - # Validate any modified api-contract.yaml with Spectral (OpenAPI 3.1). REST stack — - # no GraphQL. Skips cleanly when no contract changed. + # Validate any committed OpenAPI contract with Spectral (OpenAPI 3.1). REST stack — no + # GraphQL. Contracts are optional and ride a feature branch when present; the _example one + # is always linted. Skips cleanly when none changed. contract-validate: name: Contract Validate runs-on: ubuntu-latest @@ -92,9 +78,10 @@ jobs: set -uo pipefail base="origin/${{ github.event.pull_request.base.ref }}" git fetch --no-tags --depth=1 origin "${{ github.event.pull_request.base.ref }}" || true - changed="$(git diff --name-only "$base"...HEAD -- '.specify/features/*/api-contract.yaml' || true)" + # Any *.yaml under .specify/ or any file named like a contract. + changed="$(git diff --name-only "$base"...HEAD -- '.specify/**/*.yaml' '**/api-contract.yaml' '**/*.openapi.yaml' || true)" if [ -z "$changed" ]; then - echo "No api-contract.yaml changed — nothing to validate." + echo "No OpenAPI contract changed — nothing to validate." exit 0 fi rc=0 @@ -105,39 +92,6 @@ jobs: done exit $rc - # ─── Traceability check ─────────────────────────────────────────────────────── - # Warn (non-blocking) when a changed spec.md references a REQ-NNN that is absent from - # .specify/rtm.md. Keeps the matrix honest without hard-failing during adoption. - traceability-check: - name: Traceability Check - runs-on: ubuntu-latest - continue-on-error: true # TODO: remove to make blocking (see header) - steps: - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - - name: Cross-check spec REQ-IDs against rtm.md - shell: bash - run: | - set -uo pipefail - base="origin/${{ github.event.pull_request.base.ref }}" - git fetch --no-tags --depth=1 origin "${{ github.event.pull_request.base.ref }}" || true - changed="$(git diff --name-only "$base"...HEAD -- '.specify/features/*/spec.md' || true)" - [ -z "$changed" ] && { echo "No spec.md changed."; exit 0; } - test -f .specify/rtm.md || { echo "::warning::.specify/rtm.md missing"; exit 0; } - missing=0 - for f in $changed; do - [ -f "$f" ] || continue - for req in $(grep -oE 'REQ-[0-9]{3}' "$f" | sort -u); do - if ! grep -qF "$req" .specify/rtm.md; then - echo "::warning file=$f::$req is not present in .specify/rtm.md" - missing=$((missing+1)) - fi - done - done - echo "$missing REQ-ID(s) missing from rtm.md (warning only)." - # ─── Constitution change impact ─────────────────────────────────────────────── # When .specify/constitution.md is modified, list every file that references it (and so # may need a Sync Impact update) and post it as a PR comment. Best-effort: if no token is @@ -164,7 +118,6 @@ jobs: fi echo "changed=true" >> "$GITHUB_OUTPUT" echo "Files referencing constitution.md (review for Sync Impact):" - # rg-free: grep recursively, excluding the file itself and VCS/build dirs. grep -rIl --exclude-dir=.git --exclude-dir=node_modules --exclude-dir=target \ -e 'constitution.md' -e 'constitution §' . \ | grep -v '^\./.specify/constitution.md$' | sort > /tmp/refs.txt || true @@ -183,7 +136,6 @@ jobs: if: steps.impact.outputs.changed == 'true' shell: bash env: - # Gitea Actions exposes the repo token as GITHUB_TOKEN; the API is Gitea-compatible. TOKEN: ${{ secrets.GITHUB_TOKEN }} SERVER: ${{ github.server_url }} REPO: ${{ github.repository }} diff --git a/.specify/AGENTS.md b/.specify/AGENTS.md index 0f52319a..1b2a4df3 100644 --- a/.specify/AGENTS.md +++ b/.specify/AGENTS.md @@ -69,8 +69,9 @@ App port `8080`; management port `8081`. Backend app id: `org.raddatz.familienar ## Spec-Driven Development -Before implementing a feature, read its spec at `.specify/features//spec.md` and its -contract at `.specify/features//api-contract.yaml` if present. The spec's EARS -requirements (`REQ-NNN`) are the contract; each maps to a test. Worked reference: -[`.specify/features/_example/`](./features/_example/). Full workflow: +A feature's spec is its **Gitea issue body** — there is no committed `spec.md`. The issue's +EARS requirements (`REQ-NNN`) and acceptance criteria are the contract; each maps to a test, +traced in [`.specify/rtm.md`](./rtm.md) (`REQ-ID → issue # → test`). Read the issue before +implementing. The committed [`.specify/features/_example/`](./features/_example/) is a +template/reference showing the full artifact set, not a live feature. Full workflow: [SPEC_DRIVEN_DEVELOPMENT.md](../SPEC_DRIVEN_DEVELOPMENT.md). diff --git a/.specify/personas/developer.md b/.specify/personas/developer.md index 94481b72..45ef8f4d 100644 --- a/.specify/personas/developer.md +++ b/.specify/personas/developer.md @@ -19,12 +19,12 @@ structures or hand-wave the hard integration points. 4. Are new error conditions expressed as named `ErrorCode`s, with the four-site update (`ErrorCode.java`, `errors.ts`, `getErrorMessage()`, `messages/{de,en,es}.json`) called out as tasks? 5. Does every entity/DTO field the spec adds get `@Schema(requiredMode = REQUIRED)` where always-populated, and is `npm run generate:api` listed as a task after backend changes? 6. Are frontend changes inside the correct `$lib//` boundary, with any cross-domain import either pre-allowed in `eslint.config.js` or flagged for an explicit allow-entry? -7. Does each `REQ-NNN` map to a concrete test at the right level (unit / `@WebMvcTest` slice / Playwright E2E per COLLABORATING.md's table) in `tasks.md`? +7. Does each `REQ-NNN` imply a concrete test at the right level (unit / `@WebMvcTest` slice / Playwright E2E per COLLABORATING.md's table) — i.e. is it specified concretely enough to write that test? 8. Is lazy-loading handled — does any returned entity with a lazy collection get a view (ADR-036) instead of being serialized raw? 9. Does the design avoid premature abstraction (KISS over DRY) — no new base class/util introduced before a third caller exists? 10. Are data-model changes expressed as a single forward-only Flyway migration with the next free `V` number verified against disk? 11. Does the spec avoid backwards-compat shims for code paths that have no existing callers? -12. Is the `tasks.md` decomposition red/green-ordered — a failing test task precedes each implementation task? +12. Are the requirements decomposable into a red/green-ordered task list — each behavior small enough that a failing test can precede its implementation? ## EARS patterns to watch for diff --git a/.specify/rtm.md b/.specify/rtm.md index c3d589b7..d03e7758 100644 --- a/.specify/rtm.md +++ b/.specify/rtm.md @@ -1,19 +1,22 @@ # Requirements Traceability Matrix (RTM) -> Living document. One row per `REQ-NNN` across all in-flight and shipped features. It links -> a requirement to the design that realises it, the code that implements it, and the -> test(s) that prove it — so any requirement can be traced end to end, and any orphan -> (a requirement with no test, or a test with no requirement) is visible. +> Living document. One row per `REQ-NNN` across all in-flight and shipped features. The spec +> itself lives in the **Gitea issue** (issue-only — there is no committed `spec.md`); this +> matrix is the part of the spec that *is* committed: it links each requirement to its issue, +> the code that implements it, and the test(s) that prove it — so any requirement traces end +> to end, and any orphan (a requirement with no test) is visible on `main`. ## How to update -1. When a `spec.md` is approved, copy its `## Traceability` rows here with `Status: Planned`. -2. As tasks land, fill `Implementation File(s)` and flip `Status` → `In progress` → `Done`. -3. `REQ-ID`s are **scoped per feature**, so always qualify with the Feature column — `REQ-001` - in *avatar* is not `REQ-001` in another feature. -4. The `sdd-gate.yml` CI job (`traceability-check`) warns (non-blocking, for now) when a - `spec.md` contains a `REQ-NNN` that does not appear in this file. Keep it in sync to keep - the warning quiet; it flips to blocking once adoption settles (see the workflow's TODO). +1. When a feature's issue is approved (via `/review-issue`), add one row per `REQ-NNN` with the + `Issue` set to the Gitea issue number and `Status: Planned`. Commit these rows on the feature + branch (they merge with the feature's PR). +2. As tasks land, fill `Implementation File(s)` + `Test(s)` and flip `Status` → + `In progress` → `Done`. +3. `REQ-ID`s are **scoped per feature**, so always read them together with the `Issue` column — + `REQ-001` for issue #142 is not `REQ-001` for issue #150. +4. The `sdd-gate.yml` CI job (`rtm-check`) warns (non-blocking, for now) when a row is missing + its `Issue` or `Test(s)`. It flips to blocking once adoption settles (see the workflow's TODO). ## Status legend @@ -21,16 +24,16 @@ ## Matrix -| REQ-ID | Requirement Summary | Feature | Design Artifact | Implementation File(s) | Test(s) | Status | +| REQ-ID | Requirement Summary | Issue | Feature | Implementation File(s) | Test(s) | Status | |---|---|---|---|---|---|---| -| REQ-001 | Store avatar at `avatars/{userId}`, overwrite | profile-picture-upload (_example) | features/_example/design.md; adr-001 | `UserService` (planned) | `UserServiceAvatarTest#storesUnderUserKey`, `#replaceLeavesNoOrphan` | Planned | -| REQ-002 | Upload self avatar → 200 + avatarUrl | profile-picture-upload (_example) | features/_example/design.md; api-contract.yaml | `UserAvatarController`, `UserService` (planned) | `UserAvatarControllerTest#uploadReturnsAvatarUrl` | Planned | -| REQ-003 | Delete self avatar → avatarUrl null | profile-picture-upload (_example) | features/_example/api-contract.yaml | `UserAvatarController` (planned) | `UserAvatarControllerTest#deleteClearsAvatar` | Planned | -| REQ-004 | No avatar → null + initials placeholder | profile-picture-upload (_example) | features/_example/design.md | `UserProfileView`, avatar component (planned) | `avatar-placeholder.svelte.spec.ts` | Planned | -| REQ-005 | ADMIN_USER may delete others' avatar | profile-picture-upload (_example) | features/_example/api-contract.yaml | `UserAvatarController` (planned) | `UserAvatarControllerTest#adminDeletesOthersAvatar` | Planned | -| REQ-006 | Unauthenticated → 401, store nothing | profile-picture-upload (_example) | features/_example/threat-model.md | `SecurityConfig`, controller (planned) | `UserAvatarControllerTest#unauthenticatedReturns401` | Planned | -| REQ-007 | Non-image → 400 UNSUPPORTED_FILE_TYPE | profile-picture-upload (_example) | features/_example/design.md | `UserService` (planned) | `UserAvatarControllerTest#rejectsNonImage` | Planned | -| REQ-008 | Over 2 MB → 400 AVATAR_TOO_LARGE | profile-picture-upload (_example) | features/_example/design.md | `UserService`, `ErrorCode` (planned) | `UserAvatarControllerTest#rejectsOversize` | Planned | -| REQ-009 | Non-admin on others → 403 FORBIDDEN | profile-picture-upload (_example) | features/_example/threat-model.md | `UserAvatarController` (planned) | `UserAvatarControllerTest#nonAdminForbiddenOnOthers` | Planned | +| REQ-001 | Store avatar at `avatars/{userId}`, overwrite | #example | profile-picture-upload (_example) | `UserService` (planned) | `UserServiceAvatarTest#storesUnderUserKey`, `#replaceLeavesNoOrphan` | Planned | +| REQ-002 | Upload self avatar → 200 + avatarUrl | #example | profile-picture-upload (_example) | `UserAvatarController`, `UserService` (planned) | `UserAvatarControllerTest#uploadReturnsAvatarUrl` | Planned | +| REQ-003 | Delete self avatar → avatarUrl null | #example | profile-picture-upload (_example) | `UserAvatarController` (planned) | `UserAvatarControllerTest#deleteClearsAvatar` | Planned | +| REQ-004 | No avatar → null + initials placeholder | #example | profile-picture-upload (_example) | `UserProfileView`, avatar component (planned) | `avatar-placeholder.svelte.spec.ts` | Planned | +| REQ-005 | ADMIN_USER may delete others' avatar | #example | profile-picture-upload (_example) | `UserAvatarController` (planned) | `UserAvatarControllerTest#adminDeletesOthersAvatar` | Planned | +| REQ-006 | Unauthenticated → 401, store nothing | #example | profile-picture-upload (_example) | `SecurityConfig`, controller (planned) | `UserAvatarControllerTest#unauthenticatedReturns401` | Planned | +| REQ-007 | Non-image → 400 UNSUPPORTED_FILE_TYPE | #example | profile-picture-upload (_example) | `UserService` (planned) | `UserAvatarControllerTest#rejectsNonImage` | Planned | +| REQ-008 | Over 2 MB → 400 AVATAR_TOO_LARGE | #example | profile-picture-upload (_example) | `UserService`, `ErrorCode` (planned) | `UserAvatarControllerTest#rejectsOversize` | Planned | +| REQ-009 | Non-admin on others → 403 FORBIDDEN | #example | profile-picture-upload (_example) | `UserAvatarController` (planned) | `UserAvatarControllerTest#nonAdminForbiddenOnOthers` | Planned | - + diff --git a/.specify/templates/api-contract-stub.md b/.specify/templates/api-contract-stub.md index cf3d1f75..dac59a5b 100644 --- a/.specify/templates/api-contract-stub.md +++ b/.specify/templates/api-contract-stub.md @@ -6,19 +6,21 @@ TypeScript client from it with `npm run generate:api` (`openapi-typescript` → `frontend/src/lib/generated/api.ts`). There is no GraphQL in this stack. > **The live spec is generated from the Java controllers — it is the source of truth.** A -> hand-written contract under `.specify/features//api-contract.yaml` is a *design -> artifact*: it pins the intended shape during spec review and is checked against the -> generated spec once the endpoint exists. Keep it OpenAPI **3.1**, and keep -> `@Schema(requiredMode = REQUIRED)` on the Java side as the real driver of `required`. +> hand-written stub is a *design artifact*: it pins the intended shape during spec review. +> Issue-only: paste the stub inline into the issue's `## API / Contract Stub` section. Keep it +> OpenAPI **3.1**, and keep `@Schema(requiredMode = REQUIRED)` on the Java side as the real +> driver of `required`. ## How to use this stub -1. Copy the skeleton below to `.specify/features//api-contract.yaml`. -2. Fill in the paths/methods/schemas your feature adds. Every mutating path documents the - `403`/`401` responses and the `cookieAuth` security requirement (matching the real - `@RequirePermission` gate). -3. The `sdd-gate.yml` CI job lints any changed `api-contract.yaml` with Spectral - (`npx @stoplight/spectral-cli lint`). Run it locally the same way before pushing. +1. Fill in the skeleton below with the paths/methods/schemas your feature adds, and paste it + into the issue's `## API / Contract Stub` section. +2. Every mutating path documents the `403`/`401` responses and the `cookieAuth` security + requirement (matching the real `@RequirePermission` gate). +3. If you prefer a standalone, lintable file (e.g. for a large contract), commit it on the + **feature branch** as `.openapi.yaml` — the `sdd-gate.yml` CI job lints any + committed OpenAPI contract with Spectral (`npx @stoplight/spectral-cli lint`). It never + needs to predate the issue. 4. After the endpoint ships, run `npm run generate:api` and diff the generated types against this contract; reconcile any drift (the generated spec wins — update the contract). diff --git a/.specify/templates/feature-spec.md b/.specify/templates/feature-spec.md index 4eb61303..1c611df4 100644 --- a/.specify/templates/feature-spec.md +++ b/.specify/templates/feature-spec.md @@ -1,10 +1,10 @@ # I want so "> @@ -13,10 +13,10 @@ -Constitution principles this feature depends on: -- [§ ](../../constitution.md#) — +Constitution principles this feature depends on (see `.specify/constitution.md`): +- § — -Related: . +Related: . ## User Journey @@ -51,7 +51,7 @@ Related: . ## API / Contract Stub - + ## Data Model Changes @@ -59,7 +59,7 @@ Related: . ## Security Considerations - + ## Open Questions @@ -75,7 +75,7 @@ Related: . | REQ-001 | | | Planned | | REQ-002 | | | Planned | - + ## Persona Review Results diff --git a/.specify/templates/threat-model.md b/.specify/templates/threat-model.md index db063555..6934a3a0 100644 --- a/.specify/templates/threat-model.md +++ b/.specify/templates/threat-model.md @@ -1,12 +1,14 @@ # Threat Model — -**Feature spec:** [./spec.md](./spec.md) +**Feature spec:** Gitea issue # **Date:** **Author:** diff --git a/CLAUDE.md b/CLAUDE.md index 1181c41c..b424a47f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -18,7 +18,7 @@ See [CODESTYLE.md](./CODESTYLE.md) for coding standards: Clean Code, DRY/KISS tr ## Spec-Driven Development -This project uses Spec-Driven Development. **Before implementing a feature, read [`.specify/AGENTS.md`](./.specify/AGENTS.md)** (the short, machine-readable agent rules) and obey the [`.specify/constitution.md`](./.specify/constitution.md) it references. A feature's contract is its `.specify/features//spec.md` (EARS `REQ-NNN` requirements) plus any `api-contract.yaml`. Full workflow: [SPEC_DRIVEN_DEVELOPMENT.md](./SPEC_DRIVEN_DEVELOPMENT.md); worked example: [`.specify/features/_example/`](./.specify/features/_example/). The LLM reminders below restate constitution rules — the constitution and AGENTS.md are authoritative if they ever diverge. +This project uses Spec-Driven Development. **Before implementing a feature, read [`.specify/AGENTS.md`](./.specify/AGENTS.md)** (the short, machine-readable agent rules) and obey the [`.specify/constitution.md`](./.specify/constitution.md) it references. A feature's contract is its **Gitea issue body** (EARS `REQ-NNN` requirements) — there is no committed `spec.md`; the RTM ([`.specify/rtm.md`](./.specify/rtm.md)) traces each `REQ-ID → issue # → test`. Full workflow: [SPEC_DRIVEN_DEVELOPMENT.md](./SPEC_DRIVEN_DEVELOPMENT.md); template/reference: [`.specify/features/_example/`](./.specify/features/_example/). The LLM reminders below restate constitution rules — the constitution and AGENTS.md are authoritative if they ever diverge. --- diff --git a/SPEC_DRIVEN_DEVELOPMENT.md b/SPEC_DRIVEN_DEVELOPMENT.md index 1440bb52..ac942538 100644 --- a/SPEC_DRIVEN_DEVELOPMENT.md +++ b/SPEC_DRIVEN_DEVELOPMENT.md @@ -17,17 +17,28 @@ multi-persona review → red/green TDD). It does not replace any of that — see | # | Step | Who | Artifacts created / touched | |---|---|---|---| | 1 | **Idea → Gitea issue** using the Feature template | author | Gitea issue (labels `spec-required`, `needs-review`) from `.gitea/ISSUE_TEMPLATE/feature.md` | -| 2 | **Write the spec** — Context, User Journey, EARS `REQ-NNN` requirements, measurable acceptance criteria, Out of Scope | author | issue body **and** `.specify/features//spec.md` | -| 3 | **Add design artifacts** as needed | author | `design.md`; `api-contract.yaml` (any new endpoint); `threat-model.md` (uploads / new mutating endpoint / AI tool); feature-local `adr-NNN-*.md` or a `docs/adr/` entry for project-wide decisions | -| 4 | **Persona spec review** — the six checklists gate the spec | RE, Developer, Security, DevOps, UI/UX, Architect | `checklist-results.md` + the `## Persona Review Results` table; findings folded into the spec | -| 5 | **Resolve Open Questions & blocking FAILs** — spec does not proceed while any remain | author | spec updated; `Open Questions` emptied | -| 6 | **Decompose into tasks** in red/green order; seed the RTM | author | `tasks.md`; rows added to [`.specify/rtm.md`](./.specify/rtm.md) (`Status: Planned`) | -| 7 | **Implement** in a worktree, TDD per task (failing test → green → refactor → commit); agent reads `AGENTS.md` + `spec.md` + `api-contract.yaml` | implementer (often an AI agent) | code + tests; `npm run generate:api` after backend changes; RTM `Status` → `Done` | -| 8 | **PR → multi-persona PR review → merge**; archive the feature | reviewers | PR (`Closes #n`); on merge, move the feature dir under `.specify/features/_archive//` (or tag it shipped) | +| 2 | **Write the spec _in the issue body_** — Context, User Journey, EARS `REQ-NNN` requirements, measurable acceptance criteria, Out of Scope | author | the Gitea issue body **is** the spec (single source of truth — no committed `spec.md`) | +| 3 | **Capture durable design decisions** as needed | author | a `docs/adr/` ADR for any project-wide/irreversible decision; an OpenAPI contract and a STRIDE threat model inline in the issue (use the `.specify/templates/` as the writing aid) | +| 4 | **Persona spec review** — the six checklists gate the spec | RE, Developer, Security, DevOps, UI/UX, Architect | `/review-issue` posts each persona's checklist verdict as a Gitea comment; findings folded into the issue body | +| 5 | **Resolve Open Questions & blocking FAILs** — spec does not proceed while any remain | author | issue body updated; `Open Questions` emptied | +| 6 | **Seed the RTM** — one row per `REQ-NNN`, pointing at the issue | author | rows added to [`.specify/rtm.md`](./.specify/rtm.md) (`Issue: #n`, `Status: Planned`) — committed with the feature branch | +| 7 | **Implement** in a worktree, TDD per task (failing test → green → refactor → commit); agent reads `AGENTS.md` + the **issue body** (the spec) | implementer (often an AI agent) | code + tests; `npm run generate:api` after backend changes; RTM `Status` → `Done` | +| 8 | **PR → multi-persona PR review → merge** | reviewers | PR (`Closes #n`); the closed issue is the archived spec, the RTM rows record what shipped | -The personas at step 4 review the **spec**; the same personas at step 8 (via the existing -`review-pr` / `deliver-issue` skills) review the **code**. Step 4 catches at spec time what -used to surface only at step 8. +The personas at step 4 review the **spec (the issue)**; the same personas at step 8 (via the +existing `review-pr` / `deliver-issue` skills) review the **code**. Step 4 catches at spec time +what used to surface only at step 8. + +**Skills that drive this:** `/draft-spec` (requirements engineer authors steps 1–2 → creates +the issue) → `/review-issue` (step 4 gate) → `/implement` (steps 6–7) → `/review-pr` (step 8). +`/deliver-issue` runs review → discuss → implement → review-loop end-to-end. + +> **Why issue-only?** The Gitea issue body is the single source of truth for a spec — there is +> no committed per-feature `spec.md` to drift out of sync with it. The only SDD artifact that +> lives in git per feature is the RTM row (`REQ-ID → issue # → test`). The worked example under +> [`.specify/features/_example/`](./.specify/features/_example/) is a **template/reference**, not +> a live feature — it shows the full artifact set in one place; real features keep the spec in +> the issue. ## 2. How a Gitea issue becomes a spec @@ -63,7 +74,7 @@ measurable acceptance criterion. See the full version in ## 3. How to run a persona review Each persona reads the spec, walks its checklist in `.specify/personas/.md`, and -posts a Gitea comment (or fills `checklist-results.md`) with **PASS / FAIL / QUESTION** per +posts a Gitea comment with **PASS / FAIL / QUESTION** per item and a verdict. A `FAIL` from Security or Architect is a hard block. Concrete example: > ### Security — Spec Review @@ -86,15 +97,16 @@ empties the finding, and the persona re-reviews until `APPROVE`. This mirrors th Once the spec is `APPROVE`d and tasks are seeded, the implementer points the agent at the artifacts. Example prompt: -> Implement `.specify/features/profile-picture-upload/`. Read `.specify/AGENTS.md` and obey -> the constitution it references. The contract is `spec.md` (REQ-001…REQ-009) and -> `api-contract.yaml`. Work through `tasks.md` in order, red/green TDD — write the failing -> test named in each task first, confirm it fails, then make it pass. After backend model -> changes run `npm run generate:api`. Each REQ has a test in the Traceability table; do not -> mark a task done until its test is green. Update `.specify/rtm.md` Status as you go. +> Implement Gitea issue #142 (profile picture upload). Read `.specify/AGENTS.md` and obey the +> constitution it references. The contract is the issue body — its EARS requirements +> REQ-001…REQ-009 and acceptance criteria. Build a red/green task list from them, write the +> failing test for each REQ first, confirm it fails, then make it pass. After backend model +> changes run `npm run generate:api`. Do not mark a REQ done until its test is green; flip its +> row in `.specify/rtm.md` to Done as you go. -The agent now has: the rules (`AGENTS.md` → constitution), the exact requirements with ids, -the API shape, and a test-first task list — so its output is bounded and verifiable. +The agent now has: the rules (`AGENTS.md` → constitution) and the exact requirements with ids +from the issue — so its output is bounded and verifiable. (The `/implement` skill fetches the +issue body for you via the Gitea API.) ## 5. Maintenance rules @@ -105,13 +117,14 @@ the API shape, and a test-first task list — so its output is bounded and verif in ADR-041's revision log (or a superseding ADR for MAJOR). - **AGENTS.md** — keep it under 200 lines. It cross-references the constitution; it must never duplicate or contradict it. -- **ADRs** — project-wide decisions go in [`docs/adr/`](./docs/adr/) (next free `NNN`, verify - on disk). Immutable once `Accepted`; supersede, don't edit. Feature-local decisions stay - beside the feature spec. -- **Feature specs** — archive on merge: move `.specify/features//` to - `.specify/features/_archive//`. The spec stays as the record of what shipped. -- **RTM** ([`.specify/rtm.md`](./.specify/rtm.md)) — append rows when a spec is approved; - flip `Status` as tests go green; never delete a shipped requirement's row. CI warns on drift. +- **ADRs** — project-wide/irreversible decisions go in [`docs/adr/`](./docs/adr/) (next free + `NNN`, verify on disk). Immutable once `Accepted`; supersede, don't edit. +- **Feature specs** — the spec is the Gitea issue body; there is no committed `spec.md`. + "Archiving" is just closing the issue (`Closes #n` on merge). The closed issue + the RTM + rows are the record of what shipped. +- **RTM** ([`.specify/rtm.md`](./.specify/rtm.md)) — append one row per `REQ-NNN` when a spec + is approved, each pointing at its issue (`#n`); flip `Status` as tests go green; never delete + a shipped requirement's row. - **Personas** — update `.specify/personas/*.md` checklists when a recurring blind spot appears; keep them aligned with the richer `.claude/personas/`. @@ -133,11 +146,11 @@ the API shape, and a test-first task list — so its output is bounded and verif |---|---| | Non-negotiable rules | `.specify/constitution.md` | | Agent rules (read every time) | `.specify/AGENTS.md` | -| Templates | `.specify/templates/{feature-spec,adr,threat-model,api-contract-stub}.md` | +| Templates (writing aids) | `.specify/templates/{feature-spec,adr,threat-model,api-contract-stub}.md` | | Persona checklists | `.specify/personas/*.md` | -| In-flight feature | `.specify/features//{spec,design,tasks,checklist-results}.md` + `api-contract.yaml` + `threat-model.md` | -| Worked example | `.specify/features/_example/` | -| Traceability matrix | `.specify/rtm.md` | +| In-flight feature spec | the **Gitea issue body** (not a committed file) | +| Worked example (template/reference) | `.specify/features/_example/` | +| Traceability matrix | `.specify/rtm.md` (`REQ-ID → issue # → test`) | | ADR archive | `docs/adr/NNN-*.md` | | Issue templates | `.gitea/ISSUE_TEMPLATE/{feature,bug}.md` | | CI gate | `.gitea/workflows/sdd-gate.yml` | @@ -149,8 +162,8 @@ the API shape, and a test-first task list — so its output is bounded and verif **Commands:** ```bash -# validate a contract locally (same as CI) -npx @stoplight/spectral-cli lint .specify/features//api-contract.yaml +# validate an OpenAPI contract locally (if you drafted one — same as CI) +npx @stoplight/spectral-cli lint .yaml # regenerate the TS client after a backend model/endpoint change cd frontend && npm run generate:api # backend must run with --spring.profiles.active=dev diff --git a/docs/adr/041-sdd-adoption.md b/docs/adr/041-sdd-adoption.md index e414a418..a93d31bb 100644 --- a/docs/adr/041-sdd-adoption.md +++ b/docs/adr/041-sdd-adoption.md @@ -36,17 +36,23 @@ Adopt Spec-Driven Development, layered **on top of** the existing workflow, not 1. **EARS requirements.** Every feature requirement is written in one of the five EARS patterns and carries a per-feature `REQ-NNN` id. (constitution §3; templates/feature-spec.md) -2. **OpenSpec-style delta artifacts.** Each in-flight feature gets a `.specify/features//` - directory holding `spec.md`, `design.md`, optional `api-contract.yaml` / `threat-model.md` - / feature-local ADRs, `tasks.md`, and `checklist-results.md`. Artifacts are deltas focused - on one change, archived when the feature ships. +2. **The spec lives in the Gitea issue body — issue-only, no committed `spec.md`.** A feature's + spec (EARS requirements, acceptance criteria, scope) is authored and reviewed in the Gitea + issue, the single source of truth (consistent with the established "issue body is the source + of truth / don't commit standalone spec files" practice). The only per-feature artifact in + git is the RTM row (`REQ-ID → issue # → test`). Durable design decisions still get a + `docs/adr/` ADR; an OpenAPI contract or STRIDE threat model is drafted inline in the issue + using the `.specify/templates/` as writing aids. `.specify/features/_example/` is a committed + template/reference, not a live feature. 3. **A constitution + AGENTS.md.** `.specify/constitution.md` records the few non-negotiable rules (semantically versioned); `.specify/AGENTS.md` is the short machine-readable file AI agents read every invocation, cross-referencing the constitution rather than duplicating it. 4. **Persona checklists.** `.specify/personas/*.md` turn the existing rich personas into concise, EARS-aware, pass/fail spec-review checklists that gate a spec before code. -5. **Living RTM + CI gate.** `.specify/rtm.md` traces every `REQ-NNN`; `.gitea/workflows/sdd-gate.yml` - lints spec structure, validates contracts, and checks traceability (non-blocking initially). +5. **Living RTM + CI gate.** `.specify/rtm.md` traces every `REQ-NNN` to its issue and test; + `.gitea/workflows/sdd-gate.yml` validates any committed OpenAPI contract, lints any committed + spec (e.g. the `_example`), reports constitution-change impact, and surfaces RTM drift + (non-blocking initially). 6. **Reuse, don't duplicate.** ADRs stay in `docs/adr/`; persona checklists reference `.claude/personas/`; the Gitea issue templates mirror the feature-spec template. @@ -54,7 +60,7 @@ Adopt Spec-Driven Development, layered **on top of** the existing workflow, not | Option | Pros | Cons | Reason rejected | |---|---|---|---| -| Adopt SDD (EARS + delta artifacts + AGENTS.md), integrated with existing docs | Adds requirement rigor & machine-readability; reuses ADR archive and personas; incremental | Per-feature spec authoring overhead | **Chosen** | +| Adopt SDD (EARS requirements in the issue + AGENTS.md + constitution), integrated with existing docs | Adds requirement rigor & machine-readability; reuses ADR archive, personas, and the issue-as-spec practice; incremental | Per-feature spec authoring overhead | **Chosen** | | No change (keep free-prose issues + persona review) | Zero new process | Ambiguous requirements; no traceability; no single agent-facing file; blind spots found late | Leaves the exact gaps that hurt agent output | | Full GitHub Spec Kit | Mature, opinionated tooling | Heavy, opinionated structure that fights the existing Gitea/skill/persona workflow; redundant ADR/persona machinery | Too much to retrofit; conflicts with what already works | | BMAD-METHOD | Rich agent-role framework | Large conceptual surface for a solo project; overlaps the existing persona system | Over-engineered for this team size | -- 2.49.1 From 6400b82e1f9e1f7021055f15e4187de535d8b1a6 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 13 Jun 2026 12:14:39 +0200 Subject: [PATCH 07/10] =?UTF-8?q?feat(sdd):=20add=20/draft-spec=20skill=20?= =?UTF-8?q?=E2=80=94=20requirements-engineer=20authors=20a=20new=20spec?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Front of the SDD funnel: the requirements engineer interviews the user, elicits EARS REQ-NNN requirements + measurable acceptance criteria (probing hard for the Unwanted-behavior clauses specs usually miss), then creates the Gitea feature issue (issue body = spec), labels it, and emits RTM rows. Authors only — hands off to /review-issue rather than self-approving. Co-Authored-By: Claude Opus 4.8 --- .claude/skills/draft-spec/SKILL.md | 99 ++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 .claude/skills/draft-spec/SKILL.md diff --git a/.claude/skills/draft-spec/SKILL.md b/.claude/skills/draft-spec/SKILL.md new file mode 100644 index 00000000..13fb620f --- /dev/null +++ b/.claude/skills/draft-spec/SKILL.md @@ -0,0 +1,99 @@ +--- +name: draft-spec +description: Requirements-engineer-led authoring of a new feature spec. Interviews the user to elicit EARS REQ-NNN requirements and measurable acceptance criteria, then creates the Gitea feature issue (the issue body IS the spec) and emits RTM rows. Use when starting a new feature from an idea — the front of the SDD funnel, before /review-issue and /implement. +--- + +# Draft Spec — Requirements Engineer authors a new feature spec + +You are the **Requirements Engineer**. Read your full persona from +[`.claude/personas/req_engineer.md`](../../personas/req_engineer.md) and adopt its voice and +priorities. Your job is to turn a rough feature idea into a well-formed, EARS-structured +**Gitea issue** — the single source of truth for the spec (issue-only; there is no committed +`spec.md`). You *author* the spec; you do **not** approve it — that's `/review-issue`'s job. + +## Argument + +A free-text feature idea, e.g. `users should be able to upload a profile picture`. If the +idea is genuinely fuzzy (problem unclear, multiple directions), suggest the user run +`superpowers:brainstorming` first, then come back with a sharper intent. + +## Phase 0 — Load the SDD ground truth + +Read before interviewing: +- [`.specify/constitution.md`](../../../.specify/constitution.md) and [`.specify/AGENTS.md`](../../../.specify/AGENTS.md) — the rules the spec must respect +- [`.specify/templates/feature-spec.md`](../../../.specify/templates/feature-spec.md) — the section structure and the five EARS patterns +- [`.specify/personas/requirements-engineer.md`](../../../.specify/personas/requirements-engineer.md) — **your own checklist; apply it as you write, not after** +- [`.specify/features/_example/spec.md`](../../../.specify/features/_example/spec.md) — what "good" looks like +- [`docs/GLOSSARY.md`](../../../docs/GLOSSARY.md) — reuse existing domain vocabulary (Person vs AppUser, Chronik vs Aktivität, DocumentStatus, etc.) + +Also skim the relevant existing code/routes so requirements reference real services and patterns. + +## Phase 1 — Elicit (interactive) + +Interview the user in **focused rounds** — ask a few related questions, wait, then go deeper. +Do not dump one giant questionnaire. Cover, in roughly this order: + +1. **Why & who** — the business motivation and the role(s) involved. Drives the issue title + `As a I want so `. +2. **User journey** — the plain-prose happy path, from the user's perspective. This bounds scope. +3. **Happy-path behaviors** — what the system does on success. Each becomes a Ubiquitous, + Event-driven, or State-driven requirement. +4. **The unwanted paths — probe hard, this is where specs fail.** For every mutating action + ask: what if the caller is unauthenticated? unauthorized? what input is invalid, and what's + the limit (size, count, length)? what's the exact response (`ErrorCode` + HTTP status)? + Each answer is an Unwanted-behavior (`If …`) requirement. (Checklist item #7 is your prompt bank.) +5. **Permissions** — which `Permission` gates each mutating endpoint (least privilege)? Each + gate is an Optional-feature (`Where …`) requirement. +6. **Data model** — new tables/columns/constraints? the next free Flyway `V` (you'll verify on disk)? +7. **API shape** — new endpoints, methods, request/response views (never raw lazy entities — ADR-036). +8. **Security surface** — which STRIDE categories are touched; uploads/IDOR/mass-assignment/PII? +9. **Out of scope** — name the nearest tempting scope creep and exclude it. +10. **Open questions** — anything you cannot decide; these block until resolved. + +Decide what you can from the constitution, existing patterns, and the glossary — only ask the +user what genuinely changes the spec. Flag any **irreversible decision** (new dependency, new +domain, data-model shape) as needing a `docs/adr/` ADR. + +## Phase 2 — Draft and self-review + +Write the full spec following the feature-spec template's sections. Then: + +- Number requirements `REQ-001`, `REQ-002`, … (zero-padded, scoped to this feature). Each uses + exactly one EARS pattern. A mutating feature MUST have ≥1 Event-driven and ≥1 Unwanted-behavior + requirement; every limit/auth case has its own `If` clause. +- Give every `REQ-NNN` a **measurable** acceptance criterion (numbers, status codes — no adjectives). +- Run your `requirements-engineer.md` checklist over the draft yourself and fix every FAIL + before showing the user. (You're allowed to block your own draft.) +- Present the full draft to the user. Refine until they confirm. **Do not create the issue + until the user approves the draft text.** + +## Phase 3 — Create the Gitea issue + +Create the issue via the Gitea MCP `issue_write` tool: +- `owner` `marcel`, `repo` `familienarchiv` +- `title`: `As a I want so ` +- `body`: the approved spec (the feature-spec sections — Context, User Journey, Requirements, + Acceptance Criteria, Out of Scope, API stub, Data Model, Security, Open Questions, + Traceability, Persona Review Results). Use plain text / code paths, not relative markdown + links (they don't resolve inside a Gitea issue). +- **Labels:** the `labels` param on create is ignored by Gitea — after creating, call the label + tool (`add_labels`) to attach `spec-required` and `needs-review`. + +## Phase 4 — Emit RTM rows + flag ADRs + +- Emit ready-to-paste [`.specify/rtm.md`](../../../.specify/rtm.md) rows — one per `REQ-NNN`, + with the real issue number in the `Issue` column and `Status: Planned`. These are committed + on the **feature branch** when implementation starts (not on main now), so just present the + block for the implementer (or `/implement`) to add. If you're already on the feature's + worktree/branch, append them to `rtm.md` directly. +- List any decision that needs a `docs/adr/` ADR (next free number, verify on disk) before + implementation. + +## Phase 5 — Hand off + +Report to the user: +- The created issue URL and number +- The requirement count and that all five EARS patterns were considered +- Any remaining `Open Questions` (blockers) and any flagged ADRs +- **Next step:** run `/review-issue ` — the six personas gate the spec. You authored it; + you don't self-approve. After it passes and Open Questions are empty, run `/implement `. -- 2.49.1 From 48d1e593261cde631313e521fd6ac04b1023e983 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 13 Jun 2026 12:16:48 +0200 Subject: [PATCH 08/10] fix(sdd): add Spectral ruleset so contract-validate passes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spectral v6 ships no implicit ruleset — the CI job exited 'no ruleset found'. Adds .spectral.yaml (extends spectral:oas, documentation-only warnings relaxed for design-time stubs), adds operation tags to the _example contract so it lints clean (0 results), and aligns the api-contract-stub note. Co-Authored-By: Claude Opus 4.8 --- .specify/features/_example/api-contract.yaml | 4 ++++ .specify/templates/api-contract-stub.md | 9 +++++---- .spectral.yaml | 15 +++++++++++++++ 3 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 .spectral.yaml diff --git a/.specify/features/_example/api-contract.yaml b/.specify/features/_example/api-contract.yaml index 19685e1d..29b50702 100644 --- a/.specify/features/_example/api-contract.yaml +++ b/.specify/features/_example/api-contract.yaml @@ -45,6 +45,7 @@ paths: /api/users/me/avatar: post: summary: Upload or replace the current user's avatar + tags: [Users] operationId: uploadMyAvatar security: - cookieAuth: [] @@ -78,6 +79,7 @@ paths: schema: { $ref: '#/components/schemas/ErrorResponse' } delete: summary: Remove the current user's avatar + tags: [Users] operationId: deleteMyAvatar security: - cookieAuth: [] @@ -95,6 +97,7 @@ paths: /api/users/{id}/avatar: get: summary: Stream a user's avatar image (authenticated proxy) + tags: [Users] operationId: getUserAvatar security: - cookieAuth: [] @@ -113,6 +116,7 @@ paths: '404': { description: User has no avatar, content: { application/json: { schema: { $ref: '#/components/schemas/ErrorResponse' } } } } delete: summary: Remove another user's avatar (admin only) + tags: [Users] operationId: deleteUserAvatar description: Requires Permission.ADMIN_USER (enforced by @RequirePermission on the controller). security: diff --git a/.specify/templates/api-contract-stub.md b/.specify/templates/api-contract-stub.md index dac59a5b..050af9f4 100644 --- a/.specify/templates/api-contract-stub.md +++ b/.specify/templates/api-contract-stub.md @@ -86,11 +86,12 @@ paths: ## Validating the contract in CI -The `sdd-gate.yml` workflow runs, on PRs that touch a `api-contract.yaml`: +The `sdd-gate.yml` `contract-validate` job lints any committed OpenAPI file changed in the PR: ```bash -npx @stoplight/spectral-cli lint .specify/features/**/api-contract.yaml +npx @stoplight/spectral-cli lint .yaml ``` -Spectral's default OpenAPI ruleset catches malformed specs, missing `operationId`s, and -undefined `$ref`s. Add a `.spectral.yaml` at the repo root to tune rules if needed. +The ruleset is `.spectral.yaml` at the repo root (extends `spectral:oas`; documentation-only +warnings relaxed for design-time stubs). Spectral auto-discovers it. It catches malformed +specs, undefined `$ref`s, and duplicate `operationId`s; tune `.spectral.yaml` to adjust. diff --git a/.spectral.yaml b/.spectral.yaml new file mode 100644 index 00000000..7c7dbf28 --- /dev/null +++ b/.spectral.yaml @@ -0,0 +1,15 @@ +# Spectral ruleset for OpenAPI contract linting (SDD api-contract files). +# Spectral v6 ships no implicit ruleset — this enables the built-in OpenAPI rules. +# Used by .gitea/workflows/sdd-gate.yml (contract-validate) and locally: +# npx @stoplight/spectral-cli lint .yaml +extends: ["spectral:oas"] + +rules: + # Design-time SDD stubs are not full published API docs — relax the documentation-completeness + # warnings that would otherwise fire on a focused contract. The structural/correctness rules + # (oas3-schema, valid $refs, duplicate operationId, etc.) stay on. + info-contact: off + info-description: off + operation-description: off + operation-tag-defined: off + oas3-unused-component: off -- 2.49.1 From 5e7d22bcade938f88253119e5980c5afb8b35c97 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 13 Jun 2026 12:19:34 +0200 Subject: [PATCH 09/10] docs(sdd): add at-a-glance workflow graph, prerequisites, and gates Consolidates the full SDD workflow into one view at the top of the guide: a Mermaid flowchart (skills, the three gates, the TDD loop), a one-time prerequisites checklist, and a gates table. Co-Authored-By: Claude Opus 4.8 --- SPEC_DRIVEN_DEVELOPMENT.md | 65 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/SPEC_DRIVEN_DEVELOPMENT.md b/SPEC_DRIVEN_DEVELOPMENT.md index ac942538..a97af88d 100644 --- a/SPEC_DRIVEN_DEVELOPMENT.md +++ b/SPEC_DRIVEN_DEVELOPMENT.md @@ -12,6 +12,71 @@ multi-persona review → red/green TDD). It does not replace any of that — see --- +## 0. The whole workflow at a glance + +```mermaid +flowchart TD + idea([Feature idea]):::start --> draft + + subgraph author["✍️ Author"] + draft[/"/draft-spec
(Requirements Engineer)"/]:::skill --> issue[("Gitea issue = the SPEC
EARS REQ-NNN + acceptance criteria")]:::spec + end + + issue --> ri[/"/review-issue"/]:::skill + ri --> g1{"GATE 1 · spec review
6 personas APPROVE?
Open Questions empty?"}:::gate + g1 -- "FAIL / question" --> amend["Amend the issue body"]:::work --> ri + g1 -- "APPROVE" --> rtm["Seed RTM rows
REQ-ID → issue #"]:::work + + rtm --> wt["Create git worktree
(pull main first)"]:::work --> impl[/"/implement"/]:::skill + + subgraph build["🔁 Build · TDD per REQ-NNN"] + impl --> red["Red: failing test"]:::work --> green["Green: minimal code"]:::work --> sync["Refactor + sync
generate:api · flip RTM → Done"]:::work --> commit["Commit · Refs #n"]:::work + commit -- "next REQ" --> red + end + + build --> pr[["Open PR · Closes #n"]]:::work --> g2{"GATE 2 · CI green?
ci.yml + sdd-gate.yml"}:::gate + g2 -- "red" --> fixci["Fix on branch"]:::work --> g2 + g2 -- "green" --> rp[/"/review-pr"/]:::skill + + rp --> g3{"GATE 3 · PR review
all personas APPROVE?
every REQ implemented + tested?
no Do-Not-Touch violation?"}:::gate + g3 -- "changes requested" --> fixpr["Fix on branch"]:::work --> rp + g3 -- "APPROVE" --> merge([Merge → main
closed issue = archived spec]):::start + + rules["📐 constitution.md + AGENTS.md
(bind every step)"]:::rules -.-> draft + rules -.-> impl + rules -.-> rp + + classDef start fill:#1d3b53,color:#fff,stroke:#1d3b53; + classDef skill fill:#e8f5f0,stroke:#3aa884,color:#13352b; + classDef gate fill:#fff3cd,stroke:#d39e00,color:#5a4500; + classDef spec fill:#eef2ff,stroke:#5b6ee1,color:#1e2a5a; + classDef work fill:#f6f6f6,stroke:#bbb,color:#222; + classDef rules fill:#fdecea,stroke:#d9534f,color:#611a15; +``` + +> `/deliver-issue` runs **GATE 1 → discuss → build → GATE 3 (loop)** end-to-end in one go. + +### Prerequisites (one-time setup) + +Before the workflow runs cleanly, confirm these exist (most ship with this repo): + +- [ ] **Gitea labels** `spec-required` and `needs-review` exist (the feature template + `/draft-spec` attach them; the `labels` create-param is ignored, so they must pre-exist). +- [ ] **Gitea MCP** server configured (`gitea`) — the skills read/write issues and PRs through it. +- [ ] **`.spectral.yaml`** at the repo root (extends `spectral:oas`) — the CI contract check needs it. +- [ ] **Personas present**: identities in [`.claude/personas/`](./.claude/personas/) + checklists in [`.specify/personas/`](./.specify/personas/). +- [ ] **`.specify/constitution.md` + `AGENTS.md`** committed on `main` (so every branch inherits them). +- [ ] **Worktrees + hooks**: new feature work goes in a `git worktree` (plus-free name); run `npm install` in `frontend/` once per worktree so the pre-commit lint hook works. + +### The three gates + +| Gate | When | Mechanism | Blocks on | +|---|---|---|---| +| **1 · Spec review** | after `/draft-spec`, before any code | `/review-issue` (6 persona checklists) | any persona `CHANGES REQUESTED`, or an unresolved `## Open Question` | +| **2 · CI** | on every PR | `ci.yml` (tests · lint · semgrep) + `sdd-gate.yml` (rtm-check · contract-validate · constitution-diff) | `ci.yml` failure (hard); `sdd-gate` jobs are non-blocking during adoption — see the workflow TODO | +| **3 · PR review** | before merge | `/review-pr` (7 personas + traceability) | any persona `Changes requested`, an unimplemented/untested `REQ-NNN`, or a constitution Do-Not-Touch violation | + +--- + ## 1. The workflow in 8 steps | # | Step | Who | Artifacts created / touched | -- 2.49.1 From 31c866406822797badccd347d4138032b486992c Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 13 Jun 2026 12:20:06 +0200 Subject: [PATCH 10/10] ci(sdd): cache and pin Spectral in contract-validate Pins @stoplight/spectral-cli@6.16.0 and caches ~/.npm keyed on that version, so Spectral is fetched once and reused across runs instead of re-downloaded each time. A version bump busts the cache key deterministically. Co-Authored-By: Claude Opus 4.8 --- .gitea/workflows/sdd-gate.yml | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/.gitea/workflows/sdd-gate.yml b/.gitea/workflows/sdd-gate.yml index 6763ddbc..697eca90 100644 --- a/.gitea/workflows/sdd-gate.yml +++ b/.gitea/workflows/sdd-gate.yml @@ -72,8 +72,19 @@ jobs: with: node-version: '24' + # Cache the npm/npx download so Spectral isn't re-fetched every run. The key is pinned to + # the exact Spectral version below, so a version bump busts the cache deterministically. + - name: Cache Spectral (npm cache) + uses: actions/cache@v4 + with: + path: ~/.npm + key: spectral-cli-6.16.0 + restore-keys: spectral-cli- + - name: Lint changed OpenAPI contracts shell: bash + env: + SPECTRAL: "@stoplight/spectral-cli@6.16.0" # pinned — keep in sync with the cache key above run: | set -uo pipefail base="origin/${{ github.event.pull_request.base.ref }}" @@ -88,7 +99,7 @@ jobs: for f in $changed; do [ -f "$f" ] || continue echo "── spectral lint $f" - npx --yes @stoplight/spectral-cli@6 lint "$f" || rc=1 + npx --yes "$SPECTRAL" lint "$f" || rc=1 done exit $rc -- 2.49.1