NEO-35: add implementation plan for skill definition registry

docs/plans/NEO-35-implementation-plan.md

@@ -0,0 +1,77 @@
+# NEO-35 — Implementation plan
+
+## Story reference
+
+| Field | Value |
+|--------|--------|
+| **Key** | NEO-35 |
+| **Title** | E2.M1: SkillDefinitionRegistry service + lookup tests |
+| **Linear** | https://linear.app/neon-sprawl/issue/NEO-35/e2m1-skilldefinitionregistry-service-lookup-tests |
+
+## Kickoff clarifications
+
+| Topic | Question | Answer |
+|--------|------------|--------|
+| Scope / sequencing | Any open decisions before planning? | **None.** Linear description, [E2.M1 — SkillDefinitionRegistry](../decomposition/modules/E2_M1_SkillDefinitionRegistry.md), and shipped **NEO-34** (`SkillDefinitionCatalog`, `SkillDefRow`, loader, DI) fix scope, acceptance, and out-of-scope items. **NEO-34** is **Done**, so the dependency chain is satisfied for implementation. |
+| Kickoff continuation | User asked to continue kickoff after board/relationship clarification | Proceed with this plan on branch `NEO-35-e2m1-skilldefinition-registry`; no additional product questions pending. |
+
+## Goal, scope, and out-of-scope
+
+**Goal:** Provide a small **`ISkillDefinitionRegistry`** (name as in Linear; implementation class name may be `SkillDefinitionRegistry`) **backed by** the startup-loaded **`SkillDefinitionCatalog`**: resolve by stable **`id`**, enumerate definitions, read **`category`**, **`displayName`**, and **`allowedXpSourceKinds`**. Surface is **registered in DI** so **NEO-36** (HTTP/read model) and **E2.M2** callers can depend on the interface. Document on the interface (and briefly in code where helpful) how **E2.M2** will use this for **grant validation** (`allowedXpSourceKinds` vs `XpGrantEvent.sourceKind`).
+
+**In scope (from Linear):**
+
+- C# types aligned with [`content/schemas/skill-def.schema.json`](../../content/schemas/skill-def.schema.json) for **prototype fields** (reuse or thinly wrap existing **`SkillDefRow`** from NEO-34 — avoid duplicating divergent models).
+- Unit tests: known prototype ids resolve; **unknown id** returns absent / false **without throwing**; assert at least one **`allowedXpSourceKinds`** expectation on a known skill.
+- XML / remarks on the public contract for **E2.M2** and future callers.
+
+**Out of scope (from Linear):**
+
+- **`UnlockRequirement`** gating logic (future slice; schema has no such field today).
+- Wiring into the XP engine (**E2.M2**).
+
+## Acceptance criteria checklist
+
+- [ ] Tests: **happy path** — known prototype `id` (e.g. from existing test fixtures / `salvage` trio) resolves with expected metadata.
+- [ ] Tests: **unknown id** — lookup indicates absence / `Try*` returns false **without throwing**.
+- [ ] Tests: **`allowedXpSourceKinds`** — at least one assertion on a known skill’s list (per E2.M1 contract).
+- [ ] **`ISkillDefinitionRegistry`** (or chosen public interface name) is **registered in DI** alongside the catalog so **NEO-36** can inject it without reaching for `SkillDefinitionCatalog` internals.
+
+## Technical approach
+
+1. **Interface:** `ISkillDefinitionRegistry` with:
+   - **`TryGet`** (or equivalent) for **id → definition** with non-throwing miss semantics.
+   - **Enumeration** of all definitions (e.g. `IEnumerable<SkillDefRow>` or read-only collection) for projections.
+   - Document that **E2.M2** must reject XP grants when `sourceKind` ∉ `AllowedXpSourceKinds` for the target skill (behavior deferred to E2.M2; this story only exposes data).
+
+2. **Implementation:** `SkillDefinitionRegistry` as a **thin adapter** over **`SkillDefinitionCatalog`** (constructor-injected singleton dependency on `SkillDefinitionCatalog`).
+
+3. **DI:** Extend **`AddSkillDefinitionCatalog`** (or adjacent extension in the same file) to register **`ISkillDefinitionRegistry`** as singleton after / with the catalog registration. No requirement to eagerly-resolve the registry in **`Program.cs`** if **`SkillDefinitionCatalog`** is already resolved (NEO-34); optional `_ = app.Services.GetRequiredService<ISkillDefinitionRegistry>()` only if we want symmetry — default **omit** unless a test or ordering reason appears.
+
+4. **Models:** Prefer exposing **`SkillDefRow`** on the interface (already matches schema fields). If a separate public DTO is needed for naming only, keep it a **record with identical fields** and map in one line — avoid a second parallel shape long-term.
+
+## Files to add
+
+| Path | Purpose |
+|------|---------|
+| `server/NeonSprawl.Server/Game/Skills/ISkillDefinitionRegistry.cs` | Public contract: lookup, enumerate, XML for E2.M2 / NEO-36. |
+| `server/NeonSprawl.Server/Game/Skills/SkillDefinitionRegistry.cs` | Singleton adapter over `SkillDefinitionCatalog`. |
+
+## Files to modify
+
+| Path | Rationale |
+|------|-----------|
+| `server/NeonSprawl.Server/Game/Skills/SkillCatalogServiceCollectionExtensions.cs` | Register `ISkillDefinitionRegistry` → `SkillDefinitionRegistry` after catalog registration. |
+| `server/NeonSprawl.Server/Game/Skills/SkillDefinitionCatalog.cs` | Optional: tighten summary comment if it still says “NEO-35 may wrap” once registry exists. |
+
+## Tests
+
+| Test file | What it covers |
+|-----------|------------------|
+| `server/NeonSprawl.Server.Tests/Game/Skills/SkillDefinitionRegistryTests.cs` | **Unit:** build a **`SkillDefinitionCatalog`** in-memory (same constructor as production) with a minimal `ById` dictionary **or** load via **`SkillDefinitionCatalogLoader`** using the existing temp-layout helper pattern from **`SkillDefinitionCatalogLoaderTests`**; assert **`TryGet`** for `salvage` (or trio) returns true and **`AllowedXpSourceKinds`** contains expected values; assert unknown id returns false and does not throw; assert enumeration count matches catalog. **AAA** labels per [csharp-style](../../.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert). |
+
+Optional (only if valuable for regression): one assertion in **`SkillDefinitionCatalogLoaderTests`** host test that **`ISkillDefinitionRegistry`** resolves from **`InMemoryWebApplicationFactory`** — not strictly required if unit tests + extension registration are sufficient.
+
+## Open questions / risks
+
+**None.** If **`SkillDefRow`** is later replaced by enums for category / source kinds, the registry interface can evolve in a follow-up issue without blocking NEO-36 string-based JSON projection.