docs(adr): T3 implementation retrospective

docs/adr/modifier-profiles.md

@@ -804,3 +804,136 @@ Aura effects are treated as **derived facts**:
   delta=+1)` grants `HpBonus +1` to all allied pieces within two squares. If a
   knight moves from distance 1 to distance 3 from that king, the bonus is
   removed on the next recomputation pass.
+
+---
+
+## T3 Implementation Retrospective
+
+What changed between the T3 plan and the shipped reality, what worked, what
+hurt, and the open work the team should pick up next.
+
+### Scope delivered
+
+All 32 implementation tasks shipped across five waves:
+
+- **Wave 1**: ADR doc + primitive types/registry + custom descriptor types.
+- **Wave 2**: 15 effect primitives (state / mechanic / advanced clusters).
+- **Wave 3**: descriptor validator + Zod schema + library persistence + apply
+  integration + multi-profile stacking.
+- **Wave 4**: server `custom-modifier.register` handler + `CustomModifierEditor`
+  primitive composer + panel kind-dropdown extension + lobby multi-profile
+  stack picker + aura recomputation hook.
+- **Wave 5**: e2e suite + this retrospective + user docs + T4 forward-design.
+
+Final test counts: 1378 unit tests + the new e2e suite, all green.
+
+### What deviated from the plan
+
+#### Trigger/conditional primitives seed facts but don't (yet) fire
+
+`on-turn-start`, `on-capture`, `on-damaged`, and `conditional` all register and
+seed `OnTurnStartHooks` / `OnCaptureHooks` / `OnDamagedHooks` /
+`ConditionalHooks` facts on the target piece — but the engine pipeline that
+SHOULD evaluate those hooks at the corresponding game phase is not wired up.
+The data flows; the runtime trigger evaluation is deferred.
+
+The fact-seeding still has user value (the inspector can show "this piece has 1
+on-capture trigger"), but the primitives currently behave as data declarations,
+not active behaviour. T22's `applyCustomDescriptor` walks `childPrimitives()`
+recursively at apply time, so nested primitives DO run when the descriptor is
+applied — what doesn't yet happen is "fire on-capture's primitives WHEN this
+piece captures".
+
+Wiring is mechanical (the engine already exposes `onAfterMove`/`onDamage` hook
+points used by the modifier-integration preset for `DamageResistance`); the
+follow-up should replicate that pattern for the four trigger attrs.
+
+#### Aura contributions land in `AuraContributions` but no consumer reads them
+
+T28 ships `computeAuraFacts` and wires it to `onAfterMove` so contributions
+recompute correctly after every move. They land on each affected piece as
+`AuraContributions: Record<targetAttr, delta>`. **However**, no engine
+subsystem currently reads that record when computing effective attribute
+values — `HpBonus` consumers see only the directly-seeded value, not the
+aura-derived addition.
+
+The infrastructure is correct (idempotent, source-move retracts contribution,
+multi-source accumulation). The next step is "effective attr read points layer
+both the direct fact AND the aura contribution" — a small but careful change
+in HP / Range / DirectionAdditions consumers.
+
+#### Multi-profile stacking is solo-only on the wire
+
+T27 ships the lobby UI for ordered profile stacks. The engine
+(`applyProfilesToSession`) already supports the stack natively. The wire
+protocol, however, still carries one `ModifierProfile` per `room.create` /
+`modifier-profile.update` payload — multiplayer rooms can use a single profile
+each. Stacking on the wire would extend `RoomCreatePayloadSchema.profile?` to
+`profiles?: ModifierProfile[]` and update the consent flow (T2-ADR-2) to apply
+to a stack. Out of scope for T3.
+
+#### Server-side semantic validation deferred
+
+The server runs Zod structural validation on incoming `custom-modifier.register`
+descriptors but does NOT validate primitive-kind-in-registry or per-primitive
+params satisfaction. Doing so would require mirroring the entire 15-primitive
+catalog into the Zod-v3 server package across the v3↔v4 schema boundary.
+
+Instead: the client validates with `validateCustomDescriptor` BEFORE sending,
+and the engine's runtime applier (`applyCustomDescriptor`) silently skips
+unknown primitive kinds on the apply path. Net effect: a malicious or buggy
+client can register a structurally-valid descriptor whose primitives quietly
+no-op. Consequence: low-severity (no incorrect game behaviour can leak to
+opponent), but a server-side semantic gate is the natural T3.1 hardening.
+
+### What worked well
+
+- **Per-engine `CustomModifierRegistry` (ADR-4) prevents cross-room leakage by
+  construction**, not by discipline. We never had to hunt down a "this
+  descriptor mysteriously appeared in another room" bug because the type
+  system makes it impossible.
+- **`childPrimitives()` as the composable recursion contract** turned the
+  "validate depth ≤ 3" and "walk for apply" requirements into one-liner
+  consumers. The same callback drives the validator's tree walk and the
+  applier's recursive descent.
+- **Zod schema as both wire validator AND structural truth** kept the
+  custom-descriptor shape from drifting between protocol layer and runtime
+  type. The single boundary cast in `parseCustomModifierDescriptor` is
+  documented and small.
+- **15 separate primitive files (one each)** kept individual change sets
+  reviewable and made `git blame` informative for each primitive's evolution.
+
+### What hurt
+
+- **Long parallel agent runs blew the tool-call cap repeatedly.** Multiple
+  Wave 2 / Wave 3 / Wave 4 batches were cancelled mid-flight after the
+  delegated agent burned 200 tool calls on verification loops without
+  committing. Reconciling partial work consumed orchestrator time we wanted
+  to spend on Wave 5.
+
+  Mitigation for future epics: prompt agents to commit incrementally (every
+  2-3 files) rather than batching all commits at the end. Even better,
+  decompose the cluster prompts further (one primitive = one delegation)
+  even at the cost of more orchestration overhead.
+
+- **`as unknown as X` in lazy schemas required a documented exception.**
+  T20's recursive `EffectPrimitiveNodeSchema` couldn't cleanly satisfy
+  `z.ZodType<EffectPrimitiveNode>` because Zod infers `kind: string` and
+  the type wants `kind: PrimitiveKind`. We kept a single boundary cast in
+  `parseCustomModifierDescriptor` rather than restructuring the type.
+
+- **The chess-side Zod v4 vs server Zod v3 mismatch** forced us to mirror
+  schemas across two packages by hand. Moving the chess and server packages
+  onto the same Zod major would simplify a lot, but is a separate migration.
+
+### Open follow-ups
+
+| Item | Effort | Priority |
+|---|---|---|
+| Wire trigger primitives (on-turn-start/on-capture/on-damaged) into the engine pipeline | Medium | High — needed before custom modifiers feel "alive" |
+| Layer `AuraContributions` into HP/Range read sites | Small | High — same |
+| Server-side semantic validation of `custom-modifier.register` payloads | Medium | Medium — current degradation is silent |
+| Multi-profile on the wire for multiplayer | Medium | Medium — solo-only is a clean stop-gap |
+| Visual editor: drag-to-reorder primitives in the tree | Small | Low — polish |
+| Visual editor: nested-tree inspector (currently JSON textarea fallback) | Medium | Low — polish |
+| `CustomModifierEditor` Playwright coverage beyond happy-path | Small | Medium |