neon-sprawl/docs/plans/NEO-34-implementation-plan.md

80 lines
7.5 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters!

This file contains ambiguous Unicode characters that may be confused with others in your current locale. If your use case is intentional and legitimate, you can safely ignore this warning. Use the Escape button to highlight these characters.

# NEO-34 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEO-34 |
| **Title** | E2.M1: Server loads SkillDef catalog at startup (fail-fast) |
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-34/e2m1-server-loads-skilldef-catalog-at-startup-fail-fast |
## Kickoff clarifications
| Topic | Question | Answer |
|--------|------------|--------|
| NEO-33 dependency / sequencing | Plan relative to NEO-33 merge? | **NEO-33 merged** — assume current `content/skills/*.json` + [`scripts/validate_content.py`](../../scripts/validate_content.py) behavior is authoritative. |
| Runtime validation strategy (mirror Python vs subprocess vs defer to implementer) | C#, subprocess, or agent picks? | **Agent default:** in-process **C#** validation mirroring `validate_content.py` (JSON Schema Draft 2020-12 + same Slice-1 id/category gate). **Rationale:** no Python runtime on minimal server/CI images; fail-fast with clear paths in exceptions/logs; drift risk mitigated by parallel test fixtures and shared constants documented next to the Python script. |
## Goal, scope, and out-of-scope
**Goal:** On host startup, load all `*.json` under the configured skills catalog directory (same layout as CI: top-level `skills` array per file), validate every row against `content/schemas/skill-def.schema.json` rules, enforce **global duplicate `id`**, apply the **NEO-33 prototype Slice 1 gate** (exact ids + category coverage), build an in-memory catalog, **log** skill count and resolved paths at **Information** (or Debug for noisy detail), and register the result in DI for **NEO-35** (lookup service). **Invalid/missing data prevents the host from listening.**
**In scope (from Linear):**
- Configurable **content / skills root** (absolute path or path relative to the servers content root / current working directory — documented in `server/README.md`).
- Fail-fast semantics aligned with [`scripts/validate_content.py`](../../scripts/validate_content.py) and [E2.M1 — SkillDefinitionRegistry](../decomposition/modules/E2_M1_SkillDefinitionRegistry.md).
- Singleton (or equivalent) registration holding loaded definitions for follow-on stories.
**Out of scope (from Linear):**
- Public HTTP API for skills (**NEO-36**).
- XP / level engine (**E2.M2**).
## Acceptance criteria checklist
- [x] Integration or host-level test: **valid** fixture directory → `WebApplicationFactory` (or equivalent) starts and catalog is resolvable from DI.
- [x] Integration or host-level test: **invalid** fixture (malformed JSON, schema violation, duplicate `id`, or Slice-1 gate failure) → host startup **fails** with an **actionable** error (message includes file path and rule hint where practical).
- [x] At success: log includes **skill count** and **catalog directory** (Information or Debug per choice above).
- [x] No silent acceptance of bad catalog at runtime (matches E2.M1 acceptance).
## Technical approach
1. **Configuration:** Add a `Content` (or `SkillCatalog`) section, e.g. `SkillsDirectory` — optional string. If unset, resolve default by walking ancestors of `AppContext.BaseDirectory` until a directory `content/skills` exists (supports `dotnet run` from `server/NeonSprawl.Server` without extra env). Document override for Docker/CI via env/config.
2. **Validation library:** Add **`JsonSchema.Net`** (or equivalent with Draft 2020-12) to `NeonSprawl.Server.csproj`; load `skill-def.schema.json` once. Replicate `validate_content.py` loop: enumerate `*.json`, parse JSON, validate each object in `skills`, duplicate detection only for schema-clean rows, then run the same **frozen id set** and **category coverage** checks as Pythons `_prototype_slice1_gate` (share the three ids as a single named constant in C#; cross-reference the Python constant in a short comment).
3. **Model:** Minimal POCOs for load-time data (`Id`, `Category`, `DisplayName`, `AllowedXpSourceKinds`, etc.) — enough for registry identity and logging; NEO-35 can refine public surface.
4. **Registration:** `AddSkillDefinitionCatalog(builder.Configuration)` registers options + `SkillDefinitionCatalog` singleton; **eager** creation after `var app = builder.Build()` via `app.Services.GetRequiredService<SkillDefinitionCatalog>()` (or a tiny `IHostedService` that only validates in `StartAsync` if we need ordering with other startup) so `app.Run()` never starts with a bad catalog.
5. **Logging:** Inject `ILogger` into loader or log from `Program` after successful resolve — include absolute resolved skills directory path and distinct skill count.
## Files to add
| Path | Purpose |
|------|---------|
| `server/NeonSprawl.Server/Game/Skills/ContentPathsOptions.cs` (or `SkillCatalogOptions.cs`) | Options type bound from configuration (`SkillsDirectory`, optional future `SchemaPath`). |
| `server/NeonSprawl.Server/Game/Skills/SkillCatalogServiceCollectionExtensions.cs` | `AddSkillDefinitionCatalog` + options validation. |
| `server/NeonSprawl.Server/Game/Skills/SkillDefinitionCatalog.cs` | Immutable in-memory result of load (e.g. dictionary by id + read-only list). |
| `server/NeonSprawl.Server/Game/Skills/SkillDefinitionCatalogLoader.cs` | File I/O + schema validation + Slice-1 gate; throws typed/`InvalidOperationException` with clear messages on failure. |
| `server/NeonSprawl.Server/Game/Skills/SkillDefRow.cs` (or similar) | DTO for one catalog row after validation. |
## Files to modify
| Path | Rationale |
|------|-----------|
| `server/NeonSprawl.Server/NeonSprawl.Server.csproj` | Add JSON Schema NuGet package; optionally `Content` link to copy or embed schema path strategy (prefer reading schema from resolved repo `content/schemas` next to skills dir, or single `ContentRoot` option — align with loader). |
| `server/NeonSprawl.Server/Program.cs` | Register catalog; **eager-resolve** after build so startup fails before listen; no new public routes. |
| `server/NeonSprawl.Server/appsettings.json` | Optional `Content` section: `SkillsDirectory` / `SkillDefSchemaPath` as empty strings (auto-discovery); override via env or user secrets per [server README](../../server/README.md#skill-catalog-contentskills-neo-34). |
| `server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs` | Set default `SkillsDirectory` / content overrides so existing tests use a **valid temp or repo-relative** catalog path (tests must not depend on undeclared cwd). |
| `server/README.md` | Document `Content:SkillsDirectory`, default discovery, and Docker/CI override. |
## Tests
| Test file | What it covers |
|-----------|------------------|
| `server/NeonSprawl.Server.Tests/Game/Skills/SkillDefinitionCatalogLoaderTests.cs` | **Unit:** loader against temp directories — valid minimal catalog passes; cases for bad `skills` type, schema error, duplicate id across files, Slice-1 wrong ids / missing category coverage; assert exception messages mention file path. **Host:** `InMemoryWebApplicationFactory` + `/health` + DI catalog resolution; `WebApplicationFactory` negative case for invalid skills directory (merged here instead of a separate `SkillCatalogStartupIntegrationTests.cs`, per kickoff plan). |
## Open questions / risks
- **Schema path vs skills path:** If only `SkillsDirectory` is configured, schema resolution should be deterministic (e.g. `content/schemas/skill-def.schema.json` under the inferred repo root parent of `skills`, or a second optional setting). Document the rule in code + README to avoid Docker surprises.
- **Test cwd:** CI runs `dotnet test` from `server/` or repo root — default discovery and test overrides must be verified in both if applicable.
None blocking beyond the above (handled in implementation).