--- name: domain-entities-review version: 2.0.0 description: '[DDD Quality] Use when you need to review domain entities and value objects for DDD design quality.' --- > **[BLOCKING]** Execute skill steps in declared order. NEVER skip, reorder, or merge steps without explicit user approval. > **[BLOCKING]** Before each step or sub-skill call, update task tracking: set `in_progress` when step starts, set `completed` when step ends. > **[BLOCKING]** Every completed/skipped step MUST include brief evidence or explicit skip reason. > **[BLOCKING]** If Task tools are unavailable, create and maintain an equivalent step-by-step plan tracker with the same status transitions. ## Quick Summary **Goal:** Detect DDD design quality violations in domain entities and value objects across any technology stack — adapting to project-specific patterns via config/reference docs discovery — so domain entities and value objects preserve invariants, aggregate boundaries, and discovered DDD conventions. **Summary:** read-this-if-nothing-else digest — the skill's main steps in order: - **Phase 0 (gate):** discover the project's real entity/VO base classes, validation API, domain exception type, failure-signalling convention, concurrency mechanism + blast radius FIRST, then **0.4 detect the modelling paradigm** (OO-mutable / type-driven-immutable / event-sourced) per aggregate — discovered conventions override every generic DDD rule. — why: wrong base classes = wrong checklist, and setter rules applied to an immutable or event-sourced model manufacture false findings. - **Phase 1:** create the report, run the mandatory high-signal grep patterns (hidden `validate()` overrides, leaked persistence/business logic, missing identity markers) BEFORE reading individual files, write every grep result immediately, categorize files (root/entity/VO/unknown). - **Phase 2:** per-file checklist **A–P** — A–L (entity-vs-VO classification, base-class compliance, VO immutability/structural equality, anemic-model detection, domain invariants, invariant→property-TC Dual-Feedback, aggregate-by-ID, navigation serialization safety, domain events, query expressions, ubiquitous language, OOP) plus **M** invariant-vs-validation ownership + failure signalling, **N** construction-vs-reconstitution, **O** event dispatch timing/outbox/domain-vs-integration contract, **P** aggregate concurrency + transaction boundary — append findings per file, NEVER batch. - **Phase 3 → 4:** holistic cross-entity synthesis in the current pass, including **3.1 model-level dimensions** (bounded-context sharing; subdomain fit — judge whether a rich model is warranted BEFORE reporting anemia), then final report with health score (`100 − (CRIT×25 + HIGH×10 + MED×3 + LOW×1)`); 10+ entity files → switch to parallel `code-reviewer` sub-agents automatically. - **Phase 5 (validation-first loop):** validate via `/why-review` gate before any fix, fix only validated findings, then restart the FULL review; a clean pass ENDS the review. Every finding needs `file:line` at confidence >80%. Close with `AskUserQuestion` next-steps. **Workflow:** 1. **Phase 0** — Discover project stack + entity/VO base classes + validation API + domain exception type + failure-signalling convention + concurrency mechanism + blast radius, then **0.4 detect modelling paradigm per aggregate** **(MANDATORY FIRST)** 2. **Phase 1** — Create report; run mandatory grep patterns BEFORE per-file reads; write results immediately; categorize files 3. **Phase 2** — Entity-by-entity DDD review (per-file checklist **A–P** + project-specific rules); append per file, never batch 4. **Phase 3** — Holistic cross-entity synthesis in the current pass, incl. **3.1 model-level dimensions** (bounded-context sharing, subdomain fit); fresh-context sub-agent only after validated fixes or explicit high-risk trigger 5. **Phase 4** — Final report: critical issues, health score, refactoring priority, recommendations 6. **Phase 5** — Why-Review self-validation gate (MANDATORY when findings exist) → validate → fix validated → restart full review until clean → `AskUserQuestion` next-steps 7. **Scale rule** — 10+ entity files → parallel `code-reviewer` sub-agents, then consolidate **Key Rules:** - MUST ATTENTION discover project base classes in Phase 0 — NEVER assume generic patterns apply — why: wrong base classes = wrong checklist. - MUST ATTENTION run mandatory grep patterns in Phase 1 BEFORE reading individual files — why: highest-signal violations surface fastest and seed the report. - MUST ATTENTION validate findings via the Phase 5 `/why-review` gate before any fix, then restart the full review after validated fixes — a clean pass ENDS the review — why: every fix invalidates the prior verdict and AI reports inherit confirmation bias. - NEVER report a finding without `file:line` evidence at confidence >80% — why: unproven findings inflate severity downstream. - MUST ATTENTION append findings per file and persist to `plans/reports/` incrementally; 10+ entity files → parallel sub-agents — why: batched writes vanish on context/budget cutoff. - MUST ATTENTION detect the modelling paradigm (0.4) before applying any setter/mutability rule, and judge subdomain fit (3.1) before reporting anemic model — NEVER flag a paradigm-appropriate or appropriately-simple design as a violation — why: uniform tactical DDD over CRUD is itself an anti-pattern, and rules written for mutable OO are meaningless against an immutable or event-sourced model. - MUST ATTENTION treat invariant and validation as different questions with different owners (entity vs boundary), and keep failure signalling consistent with the Phase 0 convention — why: collapsing them buries UX checks in entities and parks business rules in bypassable validators. **Severity Classification:** | Severity | Action | Definition | | -------- | ----------- | ---------------------------------------------------------- | | CRITICAL | Block merge | Silent runtime failure, data corruption, validation bypass | | HIGH | Must fix | Incorrect behavior, invariant gap, architectural violation | | MEDIUM | Should fix | Design debt, maintainability, likely future bug | | LOW | Nice to fix | Convention, documentation, minor clarity | --- ## Canonical Owner — Domain Entity Change Gate > This skill is the **canonical owner** of `SYNC:domain-entity-change-gate`. `/plan`, `/plan-review`, and `/changes-review` inline that gate and route here for the full checklist, so a design planned under the gate is reviewed under the same rules. The gate's 6 decision points map to this skill as: classification → **A** · invariant ownership + failure signalling → **E/M** · aggregate boundary + concurrency → **F/P** · construction vs reconstitution → **N** · events → **H/O** · test obligation → **E2**; paradigm detection is **0.4** and subdomain fit is **3.1**. > > Do NOT apply the gate as a separate pass when running this skill — Phase 2 A–P **is** the gate, in full. Record `Gate is this skill's own body — A–P checklist owns it.` — why: a second pass over the same rules duplicates findings and inflates severity counts. > > Changing an entity rule here → update `.claude/skills/shared/sync-inline-versions.md` FIRST if the rule belongs to the gate's 6 decision points, then propagate to the three consumers. NEVER edit an inlined copy directly. --- ## First Principle — Easy to Change > **Success metric of every coding decision = _future change cost_.** > DRY, SRP, abstraction, design patterns, naming, layering, tests — every technique serves one goal: **make next change cheaper**. Evaluating code, refactor, test, abstraction — ask: **does this make next change cheaper or more expensive?** - Reject "best practices" raising change cost (premature abstraction, speculative generality, leaky indirection, ceremony without payoff). - Name real enemies in findings: **coupling, hidden state, duplicated knowledge, unclear intent, irreversible decisions exposed too early**. - Simpler design easy to change beats sophisticated design that isn't. Apply this lens **before** invoking any specific rule, pattern, or checklist below — if a downstream rule raises change cost, this principle wins. --- ## Phase 0: Project Discovery + Mode Detection + Blast Radius > **MANDATORY FIRST STEP.** Phase 0 gates all other work — wrong base classes = wrong checklist. **Create `TaskCreate` tasks for all phases NOW before doing anything else:** - `[Phase 0] Project stack discovery + mode detection + blast radius` — in_progress **(FIRST)** - `[Phase 1] Collect entity files + grep patterns + create report` — pending - `[Phase 2] Entity-by-entity DDD review` — pending - `[Phase 3] Holistic synthesis and fresh-context gate` — pending - `[Phase 4] Generate final findings` — pending ### 0.1 Discover Project Stack and Entity Conventions ```bash # Check for project reference docs ls docs/project-reference/ 2>/dev/null ls docs/ 2>/dev/null | grep -i "entity\|domain\|backend\|pattern" # Detect configured build/runtime markers from project config and project-reference docs rg --files | rg "(project|package|build|config|settings|manifest)" | head -20 # Find entity/VO base classes actually used rg "class.*Entity|class.*RootEntity|class.*BaseEntity|class.*AbstractEntity" {configured-source-roots} | head -10 rg "ValueObject|Aggregate|Entity" {configured-source-roots} | head -20 rg "{configured-entity-markers}" {configured-source-roots} | head -10 ``` **Record in report (required before Phase 2):** | Convention | Discovered Value | | ----------------------------- | -------------------------------------------------------------- | | Entity base class(es) | `{class names with file:line}` | | VO base class(es) | `{class names with file:line}` | | Validation API | `{how validation done}` | | Domain exception type | `{exception class used}` | | Navigation/FK pattern | `{annotation + FK property pattern}` | | Persistence annotations | `{ORM annotations}` | | Failure-signalling convention | `{throws domain exception \| returns Result/Either \| mixed}` | | Concurrency mechanism | `{version/rowversion/etag field on roots, or none}` | | Modelling paradigm | `{OO-mutable \| type-driven/immutable \| event-sourced}` (0.4) | If project reference docs exist → read them and extract: service-specific base class requirements, documented anti-patterns, naming conventions, cross-service rules. ### 0.2 Determine Entity File Scope Apply mode-appropriate command from Mode Detection table, adapted to discovered stack. ### 0.3 Blast Radius Analysis ```bash # When .code-graph/graph.db exists python .claude/scripts/code_graph trace --direction both --json --node-mode file ``` Record: entity file count, downstream consumers, risk level. Use to prioritize review order (highest-impact first). ### 0.4 Modelling Paradigm Detection (MUST ATTENTION — gates which per-file rules apply) > Sections C/D/N assume a mutable OO entity. Applying them to an immutable or event-sourced model manufactures false findings — detect the paradigm BEFORE the checklist. — why: "no public setters" is a finding in OO code and meaningless in a model that has no setters by construction. Detect from the domain source, NEVER assume: | Paradigm | Detection signal | Checklist adaptation | | --------------------------- | ------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | **OO-mutable** (default) | Classes with private setters + state-changing methods | Full A–P checklist as written | | **Type-driven / immutable** | Sealed hierarchies, discriminated unions, records-only, `With*()`/copy-returning methods, smart constructors returning `Result` | Section C immutability applies to entities too; Section D reads "no state-mutating method returns void"; illegal-state-representability replaces runtime guards — flag a status enum + nullable per-status fields as the union that was never made | | **Event-sourced** | `apply`/`evolve`/`when` per event, `From(events)` / stream-fold reconstitution, no persisted state | Section C setter rules N/A; Section N reconstitution = the fold; Section O owns event-schema evolution; a CRUD-shaped event (`{Entity}Updated` with full payload) is a HIGH finding — it carries no business meaning | - MUST ATTENTION record the detected paradigm in the report before Phase 2 and state which sections were adapted or marked N/A — why: an unrecorded adaptation reads as a skipped check. - NEVER flag a paradigm-appropriate pattern as a violation of a rule written for another paradigm — verify against 0.4 first. - Mixed paradigms per aggregate are legitimate (event-source one aggregate, not the system) — detect per aggregate, NEVER once per repo. --- ## Phase 1: Collect Files + Grep Patterns + Create Report **Create report FIRST:** `plans/reports/domain-entities-review-{date}-{slug}.md` Initialize with: Mode, Tech Stack, Discovered Conventions, Blast Radius Summary. ### Mandatory Search Intent MUST ATTENTION run high-signal searches BEFORE reading individual files. Derive the actual roots, file globs, framework markers, and naming conventions from `docs/project-config.json` plus the repository's project-reference docs. Do not copy a source-root, extension, framework type, or folder name from this skill as if it were canonical. Search for these intent categories with the configured source roots and discovered stack syntax: - Validation methods that hide or bypass the base/domain validation path. - Relationship/navigation fields that can serialize recursively or expose internal graph structure. - Value objects with mutable public state or missing structural equality. - Domain methods throwing low-context generic errors instead of configured domain/validation errors. - Business conditionals and entity mutation leaking into a higher layer when the entity/value object owns the invariant. - Query/filter expressions placed in handlers/services when the entity, repository extension, specification, or equivalent local pattern owns them. - Entity classes missing identity markers required by the configured persistence framework. - Domain models performing direct persistence, network, or infrastructure work. Representative searches — substitute the markers and source roots discovered from `docs/project-config.json` / project-reference docs (never hardcode the examples): ```bash # Validation methods that hide or bypass the base/domain validation path rg "{configured-validation-markers}" {configured-domain-source-roots} | head -20 # Persistence/query-filter expressions or infrastructure work leaked into domain models rg "{configured-persistence-or-query-markers}" {configured-domain-source-roots} | head -20 # Business conditions / entity mutation leaked above the owning domain layer rg "{configured-business-condition-patterns}" {configured-application-source-roots} | head -20 # Entity classes missing the identity markers required by the configured persistence framework rg "{configured-identity-markers}" {configured-domain-source-roots} | head -20 ``` Write ALL grep results to report IMMEDIATELY. ### Categorize Files | Category | Definition | | -------------- | -------------------------------------------------------- | | Aggregate Root | Has dedicated repository; aggregate entry point | | Entity | Has identity; accessed/persisted through root | | Value Object | Structural equality; must be immutable | | Unknown | Plain class in domain layer without clear classification | --- ## Phase 2: Entity-by-Entity DDD Review For EACH entity/VO file: read file → append findings to report IMMEDIATELY. NEVER batch. ### Per-File Review Checklist #### A. Entity vs Value Object Classification (MUST ATTENTION) > Entity = unique identity persisting across time. VO = defined by attributes, immutable, interchangeable when equal. NEVER swap roles. - verify: does class need unique persistent identity? No → suspect VO misclassification. - flag: "snapshot at point in time" (contact at referral, price at purchase, measurement at check-in) → MUST be VO, NEVER entity. - CRITICAL if VO has primary key or repository → VO masquerading as entity. - MEDIUM if entity has 3+ scalar fields always moving together → data clump → VO candidate. - MEDIUM if entity is effectively stateless (no state changes after creation) → suspect VO. #### B. Base Class Compliance (MUST ATTENTION) > NEVER assume base class — ALWAYS use discovered values from Phase 0. Project docs override generic rules. - verify aggregate root extends project's root entity base (from Phase 0 discovery). - NEVER use root entity base for non-root child entities — child entities MUST NOT have their own repository. - verify VOs extend project's VO base class — NEVER plain POCO/POJO in domain. - verify audited entities extend audited base where audit trail required. - cross-check each entity's base class against service/module-specific requirements from reference docs. #### C. Value Object Immutability and Equality (MUST ATTENTION) > Mutable VOs are a design contradiction — they imply identity through mutation, which entities have, not VOs. - NEVER allow mutable public state on value objects. Use the immutability mechanism idiomatic to the configured language/runtime. - Parameterless/default constructor allowed when required for framework deserialization. - verify equality based on structural value — NEVER reference equality. Use the equality mechanism idiomatic to the configured language/runtime or the repository's documented value-object base pattern. - verify `validate()` overridden when VO has constraints (format, range, required). - verify factory method exists for non-trivial construction: `Create()`, `New()`, `Of()`, `From*()`. - NEVER put async operations, repository calls, or infrastructure dependencies inside VO. - Conversion/implicit operator defined when VO wraps single primitive. #### D. Encapsulation and Anemic Domain Model (MUST ATTENTION) > Anemic model = entity is data bag, all logic in handlers. Fix: move behavior to entity (lowest layer). - verify entity has at least ONE domain method when it has business rules — NEVER pure property bag. - NEVER allow direct property assignment for state transitions from outside entity — MUST use domain methods (`changeStatus()`, `approve()`,