From 6344858541a7850199ccd92ce8e2ec6aafd3b034 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 2 May 2026 19:35:57 -0400 Subject: [PATCH 1/4] NEO-34: add implementation plan for skill catalog startup load --- docs/plans/NEO-34-implementation-plan.md | 82 ++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 docs/plans/NEO-34-implementation-plan.md diff --git a/docs/plans/NEO-34-implementation-plan.md b/docs/plans/NEO-34-implementation-plan.md new file mode 100644 index 0000000..3efa414 --- /dev/null +++ b/docs/plans/NEO-34-implementation-plan.md @@ -0,0 +1,82 @@ +# 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 server’s 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 + +- [ ] Integration or host-level test: **valid** fixture directory → `WebApplicationFactory` (or equivalent) starts and catalog is resolvable from DI. +- [ ] 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). +- [ ] At success: log includes **skill count** and **catalog directory** (Information or Debug per choice above). +- [ ] 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 Python’s `_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()` (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 commented or empty `Content` section documenting `SkillsDirectory`. | +| `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. | +| `server/NeonSprawl.Server.Tests/Game/Skills/SkillCatalogStartupIntegrationTests.cs` | **Host:** `WebApplicationFactory` subclass with `ConfigureWebHost` + `UseSetting` pointing at good vs bad temp paths — good starts (`Server` accessible or health GET); bad throws on factory/server startup when host is built. | + +If a single file is preferred, merge loader unit tests + factory test into one `SkillCatalogTests.cs` with clear regions. + +## 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). From 1a9088c77c07005467370f55d63e4b0852d1916d Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 2 May 2026 19:44:10 -0400 Subject: [PATCH 2/4] NEO-34: fail-fast skill catalog load at server startup Register SkillDefinitionCatalog from content/skills with JsonSchema.Net validation mirroring validate_content.py and the prototype Slice 1 gate. Resolve paths via Content:SkillsDirectory or discovery from AppContext.BaseDirectory. Eager-load after WebApplication build so bad catalogs block startup. Add unit and host tests, pin test factories to repo skills, document config in server/README, add manual QA checklist, and extend Bruno GET /health smoke tests. --- bruno/neon-sprawl-server/core/Health.bru | 10 + docs/manual-qa/NEO-34.md | 12 ++ docs/plans/NEO-34-implementation-plan.md | 8 +- .../PostgresWebApplicationFactory.cs | 6 + .../Game/Skills/SkillCatalogTestPaths.cs | 17 ++ .../SkillDefinitionCatalogLoaderTests.cs | 199 ++++++++++++++++++ .../InMemoryWebApplicationFactory.cs | 6 + .../Game/Skills/ContentPathsOptions.cs | 19 ++ .../PrototypeSlice1SkillCatalogRules.cs | 47 +++++ .../Game/Skills/SkillCatalogPathResolution.cs | 60 ++++++ ...SkillCatalogServiceCollectionExtensions.cs | 34 +++ .../Game/Skills/SkillDefRow.cs | 8 + .../Game/Skills/SkillDefinitionCatalog.cs | 27 +++ .../Skills/SkillDefinitionCatalogLoader.cs | 181 ++++++++++++++++ .../NeonSprawl.Server.csproj | 1 + server/NeonSprawl.Server/Program.cs | 3 + server/README.md | 13 ++ 17 files changed, 647 insertions(+), 4 deletions(-) create mode 100644 docs/manual-qa/NEO-34.md create mode 100644 server/NeonSprawl.Server.Tests/Game/Skills/SkillCatalogTestPaths.cs create mode 100644 server/NeonSprawl.Server.Tests/Game/Skills/SkillDefinitionCatalogLoaderTests.cs create mode 100644 server/NeonSprawl.Server/Game/Skills/ContentPathsOptions.cs create mode 100644 server/NeonSprawl.Server/Game/Skills/PrototypeSlice1SkillCatalogRules.cs create mode 100644 server/NeonSprawl.Server/Game/Skills/SkillCatalogPathResolution.cs create mode 100644 server/NeonSprawl.Server/Game/Skills/SkillCatalogServiceCollectionExtensions.cs create mode 100644 server/NeonSprawl.Server/Game/Skills/SkillDefRow.cs create mode 100644 server/NeonSprawl.Server/Game/Skills/SkillDefinitionCatalog.cs create mode 100644 server/NeonSprawl.Server/Game/Skills/SkillDefinitionCatalogLoader.cs diff --git a/bruno/neon-sprawl-server/core/Health.bru b/bruno/neon-sprawl-server/core/Health.bru index b31f314..b39e0a5 100644 --- a/bruno/neon-sprawl-server/core/Health.bru +++ b/bruno/neon-sprawl-server/core/Health.bru @@ -9,3 +9,13 @@ get { body: none auth: none } + +tests { + test("status 200", function () { + expect(res.getStatus()).to.equal(200); + }); + + test("service identity", function () { + expect(res.getBody().service).to.equal("NeonSprawl.Server"); + }); +} diff --git a/docs/manual-qa/NEO-34.md b/docs/manual-qa/NEO-34.md new file mode 100644 index 0000000..3edcd7b --- /dev/null +++ b/docs/manual-qa/NEO-34.md @@ -0,0 +1,12 @@ +# Manual QA — NEO-34 (skill catalog at startup) + +## Prerequisites + +- Repo clone with `content/skills` and `content/schemas/skill-def.schema.json` present. + +## Checks + +- [ ] From `server/NeonSprawl.Server`, run `dotnet run` (no extra `Content` env). Startup logs an **Information** line from category `NeonSprawl.Server.Game.Skills.SkillCatalog` with the **absolute** skills directory path and **3** skills (prototype catalog). +- [ ] `GET /health` returns **200** after startup. +- [ ] Introduce a deliberate catalog error (e.g. temporarily duplicate an `id` in `content/skills/prototype_skills.json`). `dotnet run` **fails** before listening; stderr shows **Skill catalog validation failed** and a path/rule hint. Revert the edit after the check. +- [ ] Optional: set `Content__SkillsDirectory` to a temp directory with invalid JSON and confirm the same fail-fast behavior. diff --git a/docs/plans/NEO-34-implementation-plan.md b/docs/plans/NEO-34-implementation-plan.md index 3efa414..ac1215c 100644 --- a/docs/plans/NEO-34-implementation-plan.md +++ b/docs/plans/NEO-34-implementation-plan.md @@ -32,10 +32,10 @@ ## Acceptance criteria checklist -- [ ] Integration or host-level test: **valid** fixture directory → `WebApplicationFactory` (or equivalent) starts and catalog is resolvable from DI. -- [ ] 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). -- [ ] At success: log includes **skill count** and **catalog directory** (Information or Debug per choice above). -- [ ] No silent acceptance of bad catalog at runtime (matches E2.M1 acceptance). +- [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 diff --git a/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresWebApplicationFactory.cs b/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresWebApplicationFactory.cs index b8a96b6..43f8466 100644 --- a/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresWebApplicationFactory.cs +++ b/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresWebApplicationFactory.cs @@ -1,6 +1,7 @@ using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.Extensions.Configuration; +using NeonSprawl.Server.Game.Skills; namespace NeonSprawl.Server.Tests.Game.PositionState; @@ -17,6 +18,11 @@ public sealed class PostgresWebApplicationFactory : WebApplicationFactory { config.AddInMemoryCollection(new Dictionary diff --git a/server/NeonSprawl.Server.Tests/Game/Skills/SkillCatalogTestPaths.cs b/server/NeonSprawl.Server.Tests/Game/Skills/SkillCatalogTestPaths.cs new file mode 100644 index 0000000..336d1b4 --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Skills/SkillCatalogTestPaths.cs @@ -0,0 +1,17 @@ +using NeonSprawl.Server.Game.Skills; + +namespace NeonSprawl.Server.Tests.Game.Skills; + +internal static class SkillCatalogTestPaths +{ + internal static string DiscoverRepoSkillsDirectory() => + SkillCatalogPathResolution.TryDiscoverSkillsDirectory(AppContext.BaseDirectory) + ?? throw new InvalidOperationException( + "Could not discover repo content/skills from AppContext.BaseDirectory; run tests from the neon-sprawl clone."); + + internal static string DiscoverRepoSkillDefSchemaPath() => + SkillCatalogPathResolution.ResolveSkillDefSchemaPath( + DiscoverRepoSkillsDirectory(), + configuredSchemaPath: null, + contentRootPath: string.Empty); +} diff --git a/server/NeonSprawl.Server.Tests/Game/Skills/SkillDefinitionCatalogLoaderTests.cs b/server/NeonSprawl.Server.Tests/Game/Skills/SkillDefinitionCatalogLoaderTests.cs new file mode 100644 index 0000000..2deff89 --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Skills/SkillDefinitionCatalogLoaderTests.cs @@ -0,0 +1,199 @@ +using System.Net; +using System.Text; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.Extensions.DependencyInjection; +using NeonSprawl.Server.Tests; +using Microsoft.Extensions.Logging.Abstractions; +using NeonSprawl.Server.Game.Skills; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.Skills; + +public class SkillDefinitionCatalogLoaderTests +{ + private const string ValidPrototypeCatalogJson = + """ + { + "schemaVersion": 1, + "skills": [ + { + "id": "salvage", + "category": "gather", + "displayName": "Salvage", + "allowedXpSourceKinds": ["activity", "mission_reward"] + }, + { + "id": "refine", + "category": "process", + "displayName": "Refine", + "allowedXpSourceKinds": ["activity", "mission_reward", "trainer"] + }, + { + "id": "intrusion", + "category": "tech", + "displayName": "Intrusion", + "allowedXpSourceKinds": ["activity", "mission_reward", "book_or_item"] + } + ] + } + """; + + private static (string Root, string SkillsDir, string SchemaPath) CreateTempContentLayout() + { + var root = Directory.CreateTempSubdirectory("neon-sprawl-skillcat-"); + var skillsDir = Path.Combine(root.FullName, "content", "skills"); + var schemaDir = Path.Combine(root.FullName, "content", "schemas"); + Directory.CreateDirectory(skillsDir); + Directory.CreateDirectory(schemaDir); + var schemaPath = Path.Combine(schemaDir, "skill-def.schema.json"); + File.Copy(SkillCatalogTestPaths.DiscoverRepoSkillDefSchemaPath(), schemaPath, overwrite: true); + return (root.FullName, skillsDir, schemaPath); + } + + [Fact] + public void Load_ShouldSucceed_WhenCatalogMatchesPrototypeContract() + { + // Arrange + var (_, skillsDir, schemaPath) = CreateTempContentLayout(); + File.WriteAllText(Path.Combine(skillsDir, "prototype.json"), ValidPrototypeCatalogJson, Encoding.UTF8); + // Act + var catalog = SkillDefinitionCatalogLoader.Load(skillsDir, schemaPath, NullLogger.Instance); + // Assert + Assert.Equal(3, catalog.DistinctSkillCount); + Assert.Equal(1, catalog.CatalogJsonFileCount); + Assert.True(catalog.ById.ContainsKey("salvage")); + Assert.Equal("gather", catalog.ById["salvage"].Category); + } + + [Fact] + public void Load_ShouldThrow_WhenSkillsIsNotArray() + { + // Arrange + var (_, skillsDir, schemaPath) = CreateTempContentLayout(); + File.WriteAllText(Path.Combine(skillsDir, "bad.json"), """{"skills": "nope"}""", Encoding.UTF8); + // Act + var ex = Record.Exception(() => SkillDefinitionCatalogLoader.Load(skillsDir, schemaPath, NullLogger.Instance)); + // Assert + var ioe = Assert.IsType(ex); + Assert.Contains("bad.json", ioe.Message, StringComparison.Ordinal); + Assert.Contains("expected top-level 'skills' array", ioe.Message, StringComparison.Ordinal); + } + + [Fact] + public void Load_ShouldThrow_WhenRowViolatesSchema() + { + // Arrange + var (_, skillsDir, schemaPath) = CreateTempContentLayout(); + var bad = """ + { + "skills": [ + { + "id": "salvage", + "category": "gather", + "displayName": "", + "allowedXpSourceKinds": ["activity"] + } + ] + } + """; + File.WriteAllText(Path.Combine(skillsDir, "bad.json"), bad, Encoding.UTF8); + // Act + var ex = Record.Exception(() => SkillDefinitionCatalogLoader.Load(skillsDir, schemaPath, NullLogger.Instance)); + // Assert + var ioe = Assert.IsType(ex); + Assert.Contains("bad.json", ioe.Message, StringComparison.Ordinal); + Assert.Contains("Skill catalog validation failed", ioe.Message, StringComparison.Ordinal); + } + + [Fact] + public void Load_ShouldThrow_WhenDuplicateIdAcrossFiles() + { + // Arrange + var (_, skillsDir, schemaPath) = CreateTempContentLayout(); + var singleSalvage = """ + { + "skills": [ + { + "id": "salvage", + "category": "gather", + "displayName": "Salvage", + "allowedXpSourceKinds": ["activity"] + } + ] + } + """; + File.WriteAllText(Path.Combine(skillsDir, "a.json"), singleSalvage, Encoding.UTF8); + File.WriteAllText(Path.Combine(skillsDir, "b.json"), singleSalvage, Encoding.UTF8); + // Act + var ex = Record.Exception(() => SkillDefinitionCatalogLoader.Load(skillsDir, schemaPath, NullLogger.Instance)); + // Assert + var ioe = Assert.IsType(ex); + Assert.Contains("duplicate skill id", ioe.Message, StringComparison.Ordinal); + Assert.Contains("salvage", ioe.Message, StringComparison.Ordinal); + Assert.Contains("a.json", ioe.Message, StringComparison.Ordinal); + Assert.Contains("b.json", ioe.Message, StringComparison.Ordinal); + } + + [Fact] + public void Load_ShouldThrow_WhenSlice1IdsIncomplete() + { + // Arrange + var (_, skillsDir, schemaPath) = CreateTempContentLayout(); + var twoOnly = """ + { + "skills": [ + { + "id": "salvage", + "category": "gather", + "displayName": "Salvage", + "allowedXpSourceKinds": ["activity"] + }, + { + "id": "refine", + "category": "process", + "displayName": "Refine", + "allowedXpSourceKinds": ["activity"] + } + ] + } + """; + File.WriteAllText(Path.Combine(skillsDir, "partial.json"), twoOnly, Encoding.UTF8); + // Act + var ex = Record.Exception(() => SkillDefinitionCatalogLoader.Load(skillsDir, schemaPath, NullLogger.Instance)); + // Assert + var ioe = Assert.IsType(ex); + Assert.Contains("prototype Slice 1 expects exactly skill ids", ioe.Message, StringComparison.Ordinal); + } + + [Fact] + public async Task Host_ShouldResolveCatalogFromDi_WhenStartupSucceeds() + { + // Arrange — InMemoryWebApplicationFactory strips Postgres and pins Content:SkillsDirectory to the repo catalog. + await using var factory = new InMemoryWebApplicationFactory(); + using var client = factory.CreateClient(); + // Act + var response = await client.GetAsync("/health"); + var catalog = factory.Services.GetRequiredService(); + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal(3, catalog.DistinctSkillCount); + } + + [Fact] + public void Host_ShouldFailStartup_WhenCatalogDirectoryInvalid() + { + // Arrange + var badDir = Path.Combine(Path.GetTempPath(), "neon-sprawl-empty-skills-" + Guid.NewGuid().ToString("n")); + Directory.CreateDirectory(badDir); + // Act + var ex = Record.Exception(() => + { + using var factory = new WebApplicationFactory().WithWebHostBuilder(b => + b.UseSetting("Content:SkillsDirectory", badDir)); + factory.CreateClient(); + }); + // Assert + Assert.NotNull(ex); + Assert.Contains("Skill catalog validation failed", ex.ToString(), StringComparison.Ordinal); + } +} diff --git a/server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs b/server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs index d93b96b..7395778 100644 --- a/server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs +++ b/server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs @@ -6,6 +6,7 @@ using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Time.Testing; using NeonSprawl.Server.Game.AbilityInput; using NeonSprawl.Server.Game.PositionState; +using NeonSprawl.Server.Game.Skills; using Npgsql; namespace NeonSprawl.Server.Tests; @@ -18,6 +19,11 @@ public sealed class InMemoryWebApplicationFactory : WebApplicationFactory { for (var i = services.Count - 1; i >= 0; i--) diff --git a/server/NeonSprawl.Server/Game/Skills/ContentPathsOptions.cs b/server/NeonSprawl.Server/Game/Skills/ContentPathsOptions.cs new file mode 100644 index 0000000..2df659e --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/ContentPathsOptions.cs @@ -0,0 +1,19 @@ +namespace NeonSprawl.Server.Game.Skills; + +/// Configuration for loading authoring content from disk (NEO-34). +public sealed class ContentPathsOptions +{ + public const string SectionName = "Content"; + + /// + /// Optional. Absolute path, or path relative to . + /// When unset, the host walks ancestors of for a content/skills directory. + /// + public string? SkillsDirectory { get; set; } + + /// + /// Optional override for skill-def.schema.json. + /// When unset, resolved as {parent of skills directory}/schemas/skill-def.schema.json (same layout as the repo). + /// + public string? SkillDefSchemaPath { get; set; } +} diff --git a/server/NeonSprawl.Server/Game/Skills/PrototypeSlice1SkillCatalogRules.cs b/server/NeonSprawl.Server/Game/Skills/PrototypeSlice1SkillCatalogRules.cs new file mode 100644 index 0000000..5b450b7 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/PrototypeSlice1SkillCatalogRules.cs @@ -0,0 +1,47 @@ +using System.Collections.Frozen; + +namespace NeonSprawl.Server.Game.Skills; + +/// +/// Prototype Slice 1 roster gate (NEO-33), mirrored from scripts/validate_content.py +/// PROTOTYPE_SLICE1_SKILL_IDS / _prototype_slice1_gate. +/// +public static class PrototypeSlice1SkillCatalogRules +{ + /// Keep in sync with scripts/validate_content.py PROTOTYPE_SLICE1_SKILL_IDS. + public static readonly FrozenSet ExpectedSkillIds = FrozenSet.ToFrozenSet( + ["salvage", "refine", "intrusion"], + StringComparer.Ordinal); + + /// Returns a human-readable error if the Slice 1 contract fails, otherwise . + public static string? TryGetSlice1GateError( + IReadOnlyDictionary skillIdToSourceFile, + IReadOnlyDictionary skillIdToCategory) + { + var ids = skillIdToSourceFile.Keys.ToFrozenSet(StringComparer.Ordinal); + if (!ids.SetEquals(ExpectedSkillIds)) + { + return + "error: prototype Slice 1 expects exactly skill ids " + + $"[{string.Join(", ", ExpectedSkillIds.Order(StringComparer.Ordinal).Select(s => "'" + s + "'"))}], " + + $"got [{string.Join(", ", ids.Order(StringComparer.Ordinal).Select(s => "'" + s + "'"))}]"; + } + + var cats = skillIdToCategory.Values.ToHashSet(StringComparer.Ordinal); + if (!cats.Contains("gather") || !cats.Contains("tech")) + { + return + "error: prototype Slice 1 requires at least one gather and one tech skill; " + + $"categories seen: [{string.Join(", ", cats.Order(StringComparer.Ordinal).Select(c => "'" + c + "'"))}]"; + } + + if (!cats.Contains("process") && !cats.Contains("make")) + { + return + "error: prototype Slice 1 requires at least one process or make skill; " + + $"categories seen: [{string.Join(", ", cats.Order(StringComparer.Ordinal).Select(c => "'" + c + "'"))}]"; + } + + return null; + } +} diff --git a/server/NeonSprawl.Server/Game/Skills/SkillCatalogPathResolution.cs b/server/NeonSprawl.Server/Game/Skills/SkillCatalogPathResolution.cs new file mode 100644 index 0000000..e40225c --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/SkillCatalogPathResolution.cs @@ -0,0 +1,60 @@ +namespace NeonSprawl.Server.Game.Skills; + +/// Resolves catalog paths for local dev, tests, and container layouts (NEO-34). +public static class SkillCatalogPathResolution +{ + /// Walks and parents for an existing content/skills directory. + public static string? TryDiscoverSkillsDirectory(string startDirectory) + { + for (var dir = new DirectoryInfo(startDirectory); dir is not null; dir = dir.Parent) + { + var candidate = Path.Combine(dir.FullName, "content", "skills"); + if (Directory.Exists(candidate)) + return Path.GetFullPath(candidate); + } + + return null; + } + + /// + /// Resolves the skills catalog directory. + /// Empty triggers discovery from . + /// + public static string ResolveSkillsDirectory(string? configuredSkillsDirectory, string contentRootPath) + { + if (string.IsNullOrWhiteSpace(configuredSkillsDirectory)) + { + var discovered = TryDiscoverSkillsDirectory(AppContext.BaseDirectory); + if (discovered is not null) + return discovered; + + throw new InvalidOperationException( + "Content:SkillsDirectory is not set and auto-discovery failed (no ancestor of AppContext.BaseDirectory contains 'content/skills'). " + + "Set Content:SkillsDirectory in configuration or environment (e.g. Content__SkillsDirectory)."); + } + + var trimmed = configuredSkillsDirectory.Trim(); + if (Path.IsPathRooted(trimmed)) + return Path.GetFullPath(trimmed); + + return Path.GetFullPath(Path.Combine(contentRootPath, trimmed)); + } + + /// Resolves JSON Schema path for a single skill row (Draft 2020-12). + public static string ResolveSkillDefSchemaPath( + string skillsDirectory, + string? configuredSchemaPath, + string contentRootPath) + { + if (!string.IsNullOrWhiteSpace(configuredSchemaPath)) + { + var trimmed = configuredSchemaPath.Trim(); + if (Path.IsPathRooted(trimmed)) + return Path.GetFullPath(trimmed); + + return Path.GetFullPath(Path.Combine(contentRootPath, trimmed)); + } + + return Path.GetFullPath(Path.Combine(skillsDirectory, "..", "schemas", "skill-def.schema.json")); + } +} diff --git a/server/NeonSprawl.Server/Game/Skills/SkillCatalogServiceCollectionExtensions.cs b/server/NeonSprawl.Server/Game/Skills/SkillCatalogServiceCollectionExtensions.cs new file mode 100644 index 0000000..19a187c --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/SkillCatalogServiceCollectionExtensions.cs @@ -0,0 +1,34 @@ +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Options; + +namespace NeonSprawl.Server.Game.Skills; + +/// DI registration for the fail-fast skill catalog (NEO-34). +public static class SkillCatalogServiceCollectionExtensions +{ + /// Binds and registers as a singleton. + public static IServiceCollection AddSkillDefinitionCatalog(this IServiceCollection services, IConfiguration configuration) + { + services.AddOptions() + .Bind(configuration.GetSection(ContentPathsOptions.SectionName)); + + services.AddSingleton(sp => + { + var hostEnv = sp.GetRequiredService(); + var opts = sp.GetRequiredService>().Value; + var logger = sp.GetRequiredService() + .CreateLogger("NeonSprawl.Server.Game.Skills.SkillCatalog"); + + var skillsDir = SkillCatalogPathResolution.ResolveSkillsDirectory(opts.SkillsDirectory, hostEnv.ContentRootPath); + var schemaPath = SkillCatalogPathResolution.ResolveSkillDefSchemaPath( + skillsDir, + opts.SkillDefSchemaPath, + hostEnv.ContentRootPath); + + return SkillDefinitionCatalogLoader.Load(skillsDir, schemaPath, logger); + }); + + return services; + } +} diff --git a/server/NeonSprawl.Server/Game/Skills/SkillDefRow.cs b/server/NeonSprawl.Server/Game/Skills/SkillDefRow.cs new file mode 100644 index 0000000..1324513 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/SkillDefRow.cs @@ -0,0 +1,8 @@ +namespace NeonSprawl.Server.Game.Skills; + +/// One validated SkillDef row from content/skills/*.json (NEO-34). +public sealed record SkillDefRow( + string Id, + string Category, + string DisplayName, + IReadOnlyList AllowedXpSourceKinds); diff --git a/server/NeonSprawl.Server/Game/Skills/SkillDefinitionCatalog.cs b/server/NeonSprawl.Server/Game/Skills/SkillDefinitionCatalog.cs new file mode 100644 index 0000000..afbac8a --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/SkillDefinitionCatalog.cs @@ -0,0 +1,27 @@ +using System.Collections.ObjectModel; + +namespace NeonSprawl.Server.Game.Skills; + +/// In-memory skill catalog loaded at startup (NEO-34). NEO-35 may wrap with lookup services. +public sealed class SkillDefinitionCatalog +{ + public SkillDefinitionCatalog( + string skillsDirectory, + IReadOnlyDictionary byId, + int catalogJsonFileCount) + { + SkillsDirectory = skillsDirectory; + ById = new ReadOnlyDictionary(new Dictionary(byId, StringComparer.Ordinal)); + CatalogJsonFileCount = catalogJsonFileCount; + } + + /// Absolute path to the directory that was enumerated for *.json catalogs. + public string SkillsDirectory { get; } + + public IReadOnlyDictionary ById { get; } + + public int DistinctSkillCount => ById.Count; + + /// Number of *.json files under . + public int CatalogJsonFileCount { get; } +} diff --git a/server/NeonSprawl.Server/Game/Skills/SkillDefinitionCatalogLoader.cs b/server/NeonSprawl.Server/Game/Skills/SkillDefinitionCatalogLoader.cs new file mode 100644 index 0000000..bec5e07 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/SkillDefinitionCatalogLoader.cs @@ -0,0 +1,181 @@ +using System.Text; +using System.Text.Json; +using System.Text.Json.Nodes; +using Json.Schema; +using Microsoft.Extensions.Logging; + +namespace NeonSprawl.Server.Game.Skills; + +/// Loads and validates content/skills/*.json using the same rules as scripts/validate_content.py (NEO-34). +public static class SkillDefinitionCatalogLoader +{ + /// Loads catalogs from disk or throws with actionable messages. + public static SkillDefinitionCatalog Load(string skillsDirectory, string schemaPath, ILogger logger) + { + skillsDirectory = Path.GetFullPath(skillsDirectory); + schemaPath = Path.GetFullPath(schemaPath); + + var errors = new List(); + + if (!File.Exists(schemaPath)) + errors.Add($"error: missing schema file {schemaPath}"); + + if (!Directory.Exists(skillsDirectory)) + errors.Add($"error: missing directory {skillsDirectory}"); + + if (errors.Count > 0) + ThrowIfAny(errors); + + var jsonFiles = Directory.GetFiles(skillsDirectory, "*.json", SearchOption.TopDirectoryOnly) + .OrderBy(p => p, StringComparer.Ordinal) + .ToArray(); + if (jsonFiles.Length == 0) + errors.Add($"error: no JSON files under {skillsDirectory}"); + + if (errors.Count > 0) + ThrowIfAny(errors); + + var schema = JsonSchema.FromText(File.ReadAllText(schemaPath)); + var evalOptions = new EvaluationOptions { OutputFormat = OutputFormat.List }; + + var skillIdToSourceFile = new Dictionary(StringComparer.Ordinal); + var skillIdToCategory = new Dictionary(StringComparer.Ordinal); + var rows = new Dictionary(StringComparer.Ordinal); + + foreach (var path in jsonFiles) + { + JsonNode? root; + try + { + root = JsonNode.Parse(File.ReadAllText(path)); + } + catch (JsonException ex) + { + errors.Add($"error: {path}: invalid JSON: {ex.Message}"); + continue; + } + + var skillsNode = root?["skills"]; + if (skillsNode is not JsonArray skillsArray) + { + errors.Add($"error: {path}: expected top-level 'skills' array"); + continue; + } + + for (var i = 0; i < skillsArray.Count; i++) + { + var item = skillsArray[i]; + if (item is not JsonObject rowObj) + { + errors.Add($"error: {path}: skills[{i}] must be an object"); + continue; + } + + var eval = schema.Evaluate(rowObj, evalOptions); + var schemaMsgs = CollectSchemaMessages(eval, path, i).OrderBy(m => m, StringComparer.Ordinal).ToList(); + if (!eval.IsValid) + { + if (schemaMsgs.Count == 0) + schemaMsgs.Add($"error: {path} skills[{i}] (root): schema validation failed"); + + errors.AddRange(schemaMsgs); + } + + var rowSchemaErrors = schemaMsgs.Count; + + var sid = (rowObj["id"] as JsonValue)?.GetValue(); + if (sid is not null && rowSchemaErrors == 0) + { + if (skillIdToSourceFile.TryGetValue(sid, out var prevPath)) + { + errors.Add($"error: duplicate skill id '{sid}' in {prevPath} and {path}"); + continue; + } + + skillIdToSourceFile[sid] = path; + + var category = (rowObj["category"] as JsonValue)?.GetValue(); + if (category is not null) + skillIdToCategory[sid] = category; + + rows[sid] = ParseRow(rowObj); + } + } + } + + ThrowIfAny(errors); + + var slice1 = PrototypeSlice1SkillCatalogRules.TryGetSlice1GateError(skillIdToSourceFile, skillIdToCategory); + if (slice1 is not null) + { + errors.Add(slice1); + ThrowIfAny(errors); + } + + logger.LogInformation( + "Loaded skill catalog from {SkillsDirectory}: {SkillCount} skill(s) across {CatalogFileCount} JSON catalog file(s).", + skillsDirectory, + rows.Count, + jsonFiles.Length); + + return new SkillDefinitionCatalog(skillsDirectory, rows, jsonFiles.Length); + } + + private static SkillDefRow ParseRow(JsonObject rowObj) + { + var id = (rowObj["id"] as JsonValue)!.GetValue(); + var category = (rowObj["category"] as JsonValue)!.GetValue(); + var displayName = (rowObj["displayName"] as JsonValue)!.GetValue(); + var kindsArray = rowObj["allowedXpSourceKinds"] as JsonArray + ?? throw new InvalidOperationException("allowedXpSourceKinds missing"); + var kinds = new List(); + foreach (var n in kindsArray) + { + if (n is JsonValue v) + kinds.Add(v.GetValue()); + } + + return new SkillDefRow(id, category, displayName, kinds); + } + + private static List CollectSchemaMessages(EvaluationResults eval, string filePath, int index) + { + var sink = new List(); + AppendSchemaMessages(eval, filePath, index, sink); + return sink; + } + + private static void AppendSchemaMessages(EvaluationResults r, string filePath, int index, List sink) + { + if (r.HasDetails) + { + foreach (var d in r.Details!) + AppendSchemaMessages(d, filePath, index, sink); + } + + if (!r.HasErrors) + return; + + foreach (var kv in r.Errors!) + { + var loc = r.InstanceLocation?.ToString(); + if (string.IsNullOrEmpty(loc) || loc == "#") + loc = "(root)"; + + sink.Add($"error: {filePath} skills[{index}] {loc}: {kv.Key} — {kv.Value}"); + } + } + + private static void ThrowIfAny(List errors) + { + if (errors.Count == 0) + return; + + var sb = new StringBuilder(); + sb.AppendLine("Skill catalog validation failed:"); + foreach (var e in errors.OrderBy(x => x, StringComparer.Ordinal)) + sb.AppendLine(e); + + throw new InvalidOperationException(sb.ToString().TrimEnd()); + } +} diff --git a/server/NeonSprawl.Server/NeonSprawl.Server.csproj b/server/NeonSprawl.Server/NeonSprawl.Server.csproj index 73ca39d..51be18c 100644 --- a/server/NeonSprawl.Server/NeonSprawl.Server.csproj +++ b/server/NeonSprawl.Server/NeonSprawl.Server.csproj @@ -19,6 +19,7 @@ + diff --git a/server/NeonSprawl.Server/Program.cs b/server/NeonSprawl.Server/Program.cs index 70ddda9..3a645f9 100644 --- a/server/NeonSprawl.Server/Program.cs +++ b/server/NeonSprawl.Server/Program.cs @@ -1,6 +1,7 @@ using NeonSprawl.Server.Game.AbilityInput; using NeonSprawl.Server.Game.Interaction; using NeonSprawl.Server.Game.PositionState; +using NeonSprawl.Server.Game.Skills; using NeonSprawl.Server.Game.Targeting; var builder = WebApplication.CreateBuilder(args); @@ -8,8 +9,10 @@ builder.Services.AddPositionStateStore(builder.Configuration); builder.Services.AddHotbarLoadoutStore(builder.Configuration); builder.Services.AddAbilityCooldownStore(); builder.Services.AddSingleton(); +builder.Services.AddSkillDefinitionCatalog(builder.Configuration); var app = builder.Build(); +_ = app.Services.GetRequiredService(); app.MapGet("/", () => Results.Text( "Neon Sprawl game server — GET /health for JSON status.", diff --git a/server/README.md b/server/README.md index a23eb52..787decb 100644 --- a/server/README.md +++ b/server/README.md @@ -24,6 +24,19 @@ NEON_SPRAWL_API_LOG=1 dotnet run --project server/NeonSprawl.Server/NeonSprawl.S IDE: use launch profile **`http+apiLog`** (same URL as **`http`**, sets the variable). +## Skill catalog (`content/skills`, NEO-34) + +On startup the host loads every **`*.json`** under the skills directory, validates each row against **`content/schemas/skill-def.schema.json`**, rejects **duplicate `id`** values across files, and enforces the **prototype Slice 1** roster gate (same rules as **`scripts/validate_content.py`**). If anything is missing or invalid, the process **exits during startup** with an actionable error—there is no silent fallback. + +| Config | Meaning | +|--------|---------| +| **`Content:SkillsDirectory`** | Optional. Absolute path, or path relative to the server **content root** (the project directory for `dotnet run`). When unset, the host walks **ancestors of `AppContext.BaseDirectory`** until it finds a **`content/skills`** directory (works for `dotnet run` from `server/NeonSprawl.Server` without extra config). | +| **`Content:SkillDefSchemaPath`** | Optional override for **`skill-def.schema.json`**. When unset, resolved as **`{parent of skills directory}/schemas/skill-def.schema.json`** (repo layout: `content/schemas` next to `content/skills`). | + +**Docker / CI:** mount or copy the repo **`content/`** tree (at least **`content/skills`** and **`content/schemas`**) and set **`Content__SkillsDirectory`** / **`Content__SkillDefSchemaPath`** if the layout differs from the default discovery rule. + +On success, **Information** logs include the resolved skills directory path and distinct skill count. + ## Position persistence (NEO-8) When **`ConnectionStrings:NeonSprawl`** is set to a valid PostgreSQL connection string, the server uses the **`player_position`** table (relational columns `pos_x` … `sequence`, not JSONB). DDL lives in [`server/db/migrations/V001__player_position.sql`](../db/migrations/V001__player_position.sql); the app applies that script on startup and on first store use (idempotent `CREATE TABLE IF NOT EXISTS`). The configured dev player (`Game:DevPlayerId` / `Game:DefaultPosition`) is seeded once at host startup, matching the in-memory store’s constructor seed (not on every HTTP request). From 922e81eb0f141afceb5169cf381da3c77550e302 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 2 May 2026 19:47:45 -0400 Subject: [PATCH 3/4] NEO-34: add code review for skill catalog startup branch --- docs/reviews/2026-05-02-NEO-34.md | 46 +++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 docs/reviews/2026-05-02-NEO-34.md diff --git a/docs/reviews/2026-05-02-NEO-34.md b/docs/reviews/2026-05-02-NEO-34.md new file mode 100644 index 0000000..cb18810 --- /dev/null +++ b/docs/reviews/2026-05-02-NEO-34.md @@ -0,0 +1,46 @@ +# Code review — NEO-34 (skill catalog startup) + +**Date:** 2026-05-02 + +**Scope:** Branch `NEO-34-e2m1-skill-catalog-startup` vs `origin/main` (commits through `1a9088c`); working tree review. + +**Base:** `origin/main` + +## Verdict + +Approve with nits + +## Summary + +The branch adds fail-fast loading of `content/skills/*.json` at server startup: JSON Schema validation (JsonSchema.Net), duplicate `id` detection, and a C# mirror of the NEO-33 prototype Slice 1 gate aligned with `scripts/validate_content.py`. `SkillDefinitionCatalog` is registered as a singleton and eagerly resolved in `Program.cs` so a bad catalog prevents the host from reaching a listening state. Unit tests cover the loader; an integration-style test uses `InMemoryWebApplicationFactory` and a `WebApplicationFactory` negative case for an empty skills directory. `server/README.md` documents `Content:SkillsDirectory` and schema resolution. All 105 tests in `NeonSprawl.sln` passed locally (`dotnet test NeonSprawl.sln`). + +## Documentation checked + +| Document | Result | Notes | +|----------|--------|--------| +| `docs/plans/NEO-34-implementation-plan.md` | Partially matches | Acceptance criteria and technical approach are largely implemented. The plan’s **Files to modify** still lists `appsettings.json` for an optional `Content` section; the diff does **not** change `appsettings.json` (README documents config instead—either add a commented `Content` block or amend the plan). The plan’s **Tests** table names `SkillCatalogStartupIntegrationTests.cs`; implementation merged host tests into `SkillDefinitionCatalogLoaderTests.cs`, which the plan explicitly allows as an alternative. | +| `docs/decomposition/modules/E2_M1_SkillDefinitionRegistry.md` | Matches | Load-time validation against `skill-def.schema.json`, duplicate `id`, and Slice 1 roster rules align with module intent. | +| `docs/decomposition/modules/CT_M1_ContentValidationPipeline.md` | Matches | Server-side validation complements CI `validate_content.py`; same schema and gate semantics. | +| `docs/decomposition/modules/module_dependency_register.md` | Matches | E2.M1 remains an appropriate home; no contract contradiction. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | Partially matches | The **Implementation tracking table** E2.M1 row still describes NEO-34 server load as **follow-on** only. After merge, that row should be updated to record **NEO-34 landed** (paths: `server/NeonSprawl.Server/Game/Skills/`, README section) per the doc’s rule on updating the table when slices land. | +| `docs/manual-qa/NEO-34.md` | Partially matches | Checklist content is useful, but it does not follow the project’s usual manual QA header (**Key / Title / Linear / Plan / Branch** table) described in `planning-implementation-docs.md` and exemplified by e.g. `docs/manual-qa/NEO-31.md`. | + +## Blocking issues + +(none) + +## Suggestions + +1. After merge (or in this PR): update **`documentation_and_implementation_alignment.md`** E2.M1 snapshot to include NEO-34 server load and pointers to `Game/Skills/` + README, so the tracking table matches reality. +2. Either add a short commented **`Content`** subsection to **`server/NeonSprawl.Server/appsettings.json`** as the plan described, or remove/adjust that row in **`NEO-34-implementation-plan.md`** so the plan matches what shipped. +3. Expand **`docs/manual-qa/NEO-34.md`** with the standard header table (Key, Title, Linear, Plan, Branch) and numbered sections for consistency with other story QA files. + +## Nits + +- **`bruno/neon-sprawl-server/core/Health.bru`**: Added Bruno `tests` blocks for status and `service` identity. Useful, but not called out in the NEO-34 plan; consider mentioning in the PR description or splitting if you want a strictly single-concern PR. +- **`SkillDefinitionCatalogLoaderTests.cs`**: `Host_ShouldResolveCatalogFromDi_WhenStartupSucceeds` puts `GetRequiredService()` under **Act** alongside `GetAsync("/health")`; strict AAA would treat DI resolution as **Assert** only (style only). + +## Verification + +- `dotnet test NeonSprawl.sln` (from repo root) — expect all tests green. +- Manual: follow `docs/manual-qa/NEO-34.md` after any edits to that file. From 9da5e7dbf51a29fa15a279519f6c3e49bea33d84 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 2 May 2026 19:50:02 -0400 Subject: [PATCH 4/4] NEO-34: address code review follow-ups (docs, appsettings, AAA) Update E2.M1 alignment table for landed catalog load; add Content placeholders to appsettings.json; align implementation plan tests/appsettings notes; expand manual QA header to project standard; strike review suggestions; tighten host catalog test Arrange/Act/Assert. --- ...umentation_and_implementation_alignment.md | 2 +- docs/manual-qa/NEO-34.md | 27 +++++++++++++------ docs/plans/NEO-34-implementation-plan.md | 7 ++--- docs/reviews/2026-05-02-NEO-34.md | 16 +++++------ .../SkillDefinitionCatalogLoaderTests.cs | 2 +- server/NeonSprawl.Server/appsettings.json | 4 +++ 6 files changed, 35 insertions(+), 23 deletions(-) diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index 06c55ee..a5821c7 100644 --- a/docs/decomposition/modules/documentation_and_implementation_alignment.md +++ b/docs/decomposition/modules/documentation_and_implementation_alignment.md @@ -50,7 +50,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not | E1.M2 | Ready | **Slice 2 prototype slice closed:** follow + `CameraState` ([NEO-15](https://linear.app/neon-sprawl/issue/NEO-15)); zoom bands ([NEO-16](https://linear.app/neon-sprawl/issue/NEO-16)); occlusion ([NEO-17](https://linear.app/neon-sprawl/issue/NEO-17)); occluder pick-through ([NEO-20](https://linear.app/neon-sprawl/issue/NEO-20)); contracts + hardening ([NEO-18](https://linear.app/neon-sprawl/issue/NEO-18)). Client-local; no server use of camera pose. | [NEO-15](../../plans/NEO-15-implementation-plan.md), [NEO-16](../../plans/NEO-16-implementation-plan.md), [NEO-17](../../plans/NEO-17-implementation-plan.md), [NEO-18](../../plans/NEO-18-implementation-plan.md), [NEO-20](../../plans/NEO-20-implementation-plan.md); `client/scripts/isometric_follow_camera.gd`, `camera_state.gd`, `zoom_band_config.gd`, `occlusion_policy.gd`, `ground_pick.gd`; [E1_M2_IsometricCameraController.md](E1_M2_IsometricCameraController.md) | | E1.M3 | In Progress | **NEO-23 landed:** JSON v1 targeting read + select (`GET`/`POST …/target`, `Game/Targeting/`, [NEO-23](../../plans/NEO-23-implementation-plan.md)) — `PlayerTargetStateResponse` / soft lock / `reasonCode` denials; wire name differs from illustrative **`TargetState`** in module doc (called out in plan + README). **NEO-24 landed:** client tab-target + lock HUD synced to server ([NEO-24](../../plans/NEO-24-implementation-plan.md)) — `TargetSelectionClient` ([`client/scripts/target_selection_client.gd`](../../../client/scripts/target_selection_client.gd)), Tab / Esc bindings, `TargetLockLabel` HUD, hybrid movement-triggered refresh via new `PositionAuthorityClient.authoritative_ack` signal (fires on every server-confirmed position, boot + `move-stream` 200) with a 250 ms cooldown; manual QA in [`docs/manual-qa/NEO-24.md`](../../manual-qa/NEO-24.md). **NEO-25 landed:** versioned **`GET /game/world/interactables`** ([NEO-25](../../plans/NEO-25-implementation-plan.md)) — `InteractablesListResponse` / `InteractablesWorldApi` in `server/NeonSprawl.Server/Game/Interaction/`; Godot `interactables_catalog_client.gd` + `interactable_world_builder.gd` (fetch-driven props + glow); [server README — Interactable descriptors (NEO-25)](../../../server/README.md#interactable-descriptors-neo-25). **NEO-26 landed:** prototype **`SelectionEvent`** — **`TargetSelectionClient.selection_event`** ([NEO-26](../../plans/NEO-26-implementation-plan.md)) when **`lockedTargetId`** changes; optional **`log_selection_events`**. **NEO-27 landed:** Slice 3 telemetry hook sites documented — `selection_event` maps to product `target_changed` in `target_selection_client.gd`; server lock transition hook comments in `InMemoryPlayerTargetLockStore`; reserved `ability_cast_requested` / `ability_cast_denied` comments in `client/scripts/main.gd` (all TODO(E9.M1)); see [NEO-27](../../plans/NEO-27-implementation-plan.md) and [`docs/manual-qa/NEO-27.md`](../../manual-qa/NEO-27.md). **Follow-on / still open:** richer **`InteractableDescriptor`** consumers beyond list projection + existing `POST …/interact`; production telemetry ingest/catalog after E9.M1. **Precursor (E1.M1):** [NEO-9](../../plans/NEO-9-implementation-plan.md) `POST …/interact`. | Linear [NEO-24](https://linear.app/neon-sprawl/issue/NEO-24)–[NEO-27](https://linear.app/neon-sprawl/issue/NEO-27); [E1_M3_InteractionAndTargetingLayer.md](E1_M3_InteractionAndTargetingLayer.md); [E1M3 prototype backlog](../../plans/E1M3-prototype-backlog.md); [server README — Targeting](../../../server/README.md#targeting-neo-23), [Interactable descriptors (NEO-25)](../../../server/README.md#interactable-descriptors-neo-25), [Interaction](../../../server/README.md#interaction-neo-9) | | E1.M4 | In Progress | **NEO-29 landed:** prototype `HotbarLoadout` v1 server contract (`GET`/`POST /game/players/{id}/hotbar-loadout`) with stable deny reason codes, per-player scope, and persistence policy matching NEO-8/NS-17 (Postgres when configured, in-memory fallback otherwise). Client boot hydration is wired via `hotbar_loadout_client.gd` + `hotbar_state.gd` from `main.gd`; manual QA checklist created. **NEO-31 landed:** prototype **`POST /game/players/{id}/ability-cast`**, digit-key cast wiring from `main.gd`, `ability_cast_client.gd`, and `hotbar_cast_slot_resolver.gd` (target id from `lockedTargetId` in cached target state); GdUnit covers resolver + HTTP client; manual QA [NEO-31](../../manual-qa/NEO-31.md). **NEO-28 landed:** cast POST validates **lock + registry + range** (`invalid_target`, `out_of_range`); **`AbilityCastClient.cast_result_received`** + **`CastFeedbackLabel`** HUD line; manual QA [NEO-28](../../manual-qa/NEO-28.md). **NEO-30 landed:** Slice 3 cast funnel telemetry **hook-site comments** in [`AbilityCastApi.cs`](../../../server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs) (`ability_cast_requested` on authoritative accept, `ability_cast_denied` + non-empty `reasonCode` on each JSON deny branch); client dev hooks unchanged; manual QA [NEO-30](../../manual-qa/NEO-30.md); plan [NEO-30](../../plans/NEO-30-implementation-plan.md). **NEO-32 landed:** `GET /game/players/{id}/cooldown-snapshot` + `on_cooldown` cast deny + prototype global cooldown commit; [`cooldown_snapshot_client.gd`](../../../client/scripts/cooldown_snapshot_client.gd), [`cooldown_state.gd`](../../../client/scripts/cooldown_state.gd), **`CooldownSlotsLabel`** in [`main.gd`](../../../client/scripts/main.gd); manual QA [NEO-32](../../manual-qa/NEO-32.md); plan [NEO-32](../../plans/NEO-32-implementation-plan.md). | [NEO-29](../../plans/NEO-29-implementation-plan.md), [NEO-31](../../plans/NEO-31-implementation-plan.md), [NEO-28](../../plans/NEO-28-implementation-plan.md), [NEO-30](../../plans/NEO-30-implementation-plan.md), [NEO-32](../../plans/NEO-32-implementation-plan.md); [E1_M4_AbilityInputScaffold.md](E1_M4_AbilityInputScaffold.md); [E1M4 prototype backlog](../../plans/E1M4-prototype-backlog.md); `server/NeonSprawl.Server/Game/AbilityInput/`; [`client/scripts/hotbar_loadout_client.gd`](../../../client/scripts/hotbar_loadout_client.gd), [`client/scripts/hotbar_state.gd`](../../../client/scripts/hotbar_state.gd), [`client/scripts/ability_cast_client.gd`](../../../client/scripts/ability_cast_client.gd), [`client/scripts/hotbar_cast_slot_resolver.gd`](../../../client/scripts/hotbar_cast_slot_resolver.gd), [`client/scripts/cooldown_snapshot_client.gd`](../../../client/scripts/cooldown_snapshot_client.gd), [`client/scripts/cooldown_state.gd`](../../../client/scripts/cooldown_state.gd); [server README — Hotbar loadout](../../../server/README.md#hotbar-loadout-neo-29), [Ability cast (NEO-31)](../../../server/README.md#ability-cast-neo-31), [Cooldown snapshot (NEO-32)](../../../server/README.md#cooldown-snapshot-neo-32) | -| E2.M1 | In Progress | **NEO-33 landed:** frozen prototype trio **`salvage`** / **`refine`** / **`intrusion`** in [`content/skills/prototype_skills.json`](../../../content/skills/prototype_skills.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) enforce schema, duplicate `id`, exact trio ids, and category coverage; designer note on **`allowedXpSourceKinds`** in [E2_M1](E2_M1_SkillDefinitionRegistry.md) + [`content/README.md`](../../../content/README.md). **Follow-on:** server load ([NEO-34](https://linear.app/neon-sprawl/issue/NEO-34)), registry service ([NEO-35](https://linear.app/neon-sprawl/issue/NEO-35)), read HTTP + Bruno ([NEO-36](https://linear.app/neon-sprawl/issue/NEO-36)). | [NEO-33](../../plans/NEO-33-implementation-plan.md); [E2_M1](E2_M1_SkillDefinitionRegistry.md); [module_dependency_register.md](module_dependency_register.md) **E2.M1 note** | +| E2.M1 | In Progress | **NEO-33 landed:** frozen prototype trio **`salvage`** / **`refine`** / **`intrusion`** in [`content/skills/prototype_skills.json`](../../../content/skills/prototype_skills.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) enforce schema, duplicate `id`, exact trio ids, and category coverage; designer note on **`allowedXpSourceKinds`** in [E2_M1](E2_M1_SkillDefinitionRegistry.md) + [`content/README.md`](../../../content/README.md). **NEO-34 landed:** fail-fast server load of `content/skills/*.json` at startup — `server/NeonSprawl.Server/Game/Skills/` ([NEO-34](../../plans/NEO-34-implementation-plan.md)); config + discovery in [server README — Skill catalog](../../../server/README.md#skill-catalog-contentskills-neo-34). **Follow-on:** registry service ([NEO-35](https://linear.app/neon-sprawl/issue/NEO-35)), read HTTP + Bruno ([NEO-36](https://linear.app/neon-sprawl/issue/NEO-36)). | [NEO-33](../../plans/NEO-33-implementation-plan.md), [NEO-34](../../plans/NEO-34-implementation-plan.md); [E2_M1](E2_M1_SkillDefinitionRegistry.md); [module_dependency_register.md](module_dependency_register.md) **E2.M1 note**; `server/NeonSprawl.Server/Game/Skills/` | --- diff --git a/docs/manual-qa/NEO-34.md b/docs/manual-qa/NEO-34.md index 3edcd7b..2e1391e 100644 --- a/docs/manual-qa/NEO-34.md +++ b/docs/manual-qa/NEO-34.md @@ -1,12 +1,23 @@ -# Manual QA — NEO-34 (skill catalog at startup) +# NEO-34 — Manual QA checklist -## Prerequisites +| 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 | +| Plan | `docs/plans/NEO-34-implementation-plan.md` | +| Branch | `NEO-34-e2m1-skill-catalog-startup` | -- Repo clone with `content/skills` and `content/schemas/skill-def.schema.json` present. +## 1) Happy path — discovery + health -## Checks +- [ ] Repo has `content/skills` and `content/schemas/skill-def.schema.json`. +- [ ] From `server/NeonSprawl.Server`, run `dotnet run` with **no** `Content__*` overrides. Startup logs an **Information** line from category `NeonSprawl.Server.Game.Skills.SkillCatalog` with the **absolute** skills directory path and **3** skills (prototype catalog). +- [ ] `GET /health` (or Bruno `core/Health`) returns **200** and JSON includes `service: "NeonSprawl.Server"`. -- [ ] From `server/NeonSprawl.Server`, run `dotnet run` (no extra `Content` env). Startup logs an **Information** line from category `NeonSprawl.Server.Game.Skills.SkillCatalog` with the **absolute** skills directory path and **3** skills (prototype catalog). -- [ ] `GET /health` returns **200** after startup. -- [ ] Introduce a deliberate catalog error (e.g. temporarily duplicate an `id` in `content/skills/prototype_skills.json`). `dotnet run` **fails** before listening; stderr shows **Skill catalog validation failed** and a path/rule hint. Revert the edit after the check. -- [ ] Optional: set `Content__SkillsDirectory` to a temp directory with invalid JSON and confirm the same fail-fast behavior. +## 2) Fail-fast — invalid catalog + +- [ ] Temporarily introduce a catalog error (e.g. duplicate an `id` in `content/skills/prototype_skills.json`). `dotnet run` **exits** before listening; stderr includes **Skill catalog validation failed** and a path/rule hint. **Revert** the edit after the check. + +## 3) Optional — explicit config + +- [ ] Set `Content__SkillsDirectory` to a temp directory with invalid or empty catalogs and confirm the same fail-fast behavior (see [server README — Skill catalog](../../server/README.md#skill-catalog-contentskills-neo-34)). diff --git a/docs/plans/NEO-34-implementation-plan.md b/docs/plans/NEO-34-implementation-plan.md index ac1215c..0ffba30 100644 --- a/docs/plans/NEO-34-implementation-plan.md +++ b/docs/plans/NEO-34-implementation-plan.md @@ -61,7 +61,7 @@ |------|-----------| | `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 commented or empty `Content` section documenting `SkillsDirectory`. | +| `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. | @@ -69,10 +69,7 @@ | 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. | -| `server/NeonSprawl.Server.Tests/Game/Skills/SkillCatalogStartupIntegrationTests.cs` | **Host:** `WebApplicationFactory` subclass with `ConfigureWebHost` + `UseSetting` pointing at good vs bad temp paths — good starts (`Server` accessible or health GET); bad throws on factory/server startup when host is built. | - -If a single file is preferred, merge loader unit tests + factory test into one `SkillCatalogTests.cs` with clear regions. +| `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 diff --git a/docs/reviews/2026-05-02-NEO-34.md b/docs/reviews/2026-05-02-NEO-34.md index cb18810..4698fb2 100644 --- a/docs/reviews/2026-05-02-NEO-34.md +++ b/docs/reviews/2026-05-02-NEO-34.md @@ -18,12 +18,12 @@ The branch adds fail-fast loading of `content/skills/*.json` at server startup: | Document | Result | Notes | |----------|--------|--------| -| `docs/plans/NEO-34-implementation-plan.md` | Partially matches | Acceptance criteria and technical approach are largely implemented. The plan’s **Files to modify** still lists `appsettings.json` for an optional `Content` section; the diff does **not** change `appsettings.json` (README documents config instead—either add a commented `Content` block or amend the plan). The plan’s **Tests** table names `SkillCatalogStartupIntegrationTests.cs`; implementation merged host tests into `SkillDefinitionCatalogLoaderTests.cs`, which the plan explicitly allows as an alternative. | +| `docs/plans/NEO-34-implementation-plan.md` | Matches | Acceptance criteria and technical approach match implementation; `appsettings.json` includes optional `Content` keys; **Tests** table documents merged host coverage in `SkillDefinitionCatalogLoaderTests.cs`. Done. | | `docs/decomposition/modules/E2_M1_SkillDefinitionRegistry.md` | Matches | Load-time validation against `skill-def.schema.json`, duplicate `id`, and Slice 1 roster rules align with module intent. | | `docs/decomposition/modules/CT_M1_ContentValidationPipeline.md` | Matches | Server-side validation complements CI `validate_content.py`; same schema and gate semantics. | | `docs/decomposition/modules/module_dependency_register.md` | Matches | E2.M1 remains an appropriate home; no contract contradiction. | -| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | Partially matches | The **Implementation tracking table** E2.M1 row still describes NEO-34 server load as **follow-on** only. After merge, that row should be updated to record **NEO-34 landed** (paths: `server/NeonSprawl.Server/Game/Skills/`, README section) per the doc’s rule on updating the table when slices land. | -| `docs/manual-qa/NEO-34.md` | Partially matches | Checklist content is useful, but it does not follow the project’s usual manual QA header (**Key / Title / Linear / Plan / Branch** table) described in `planning-implementation-docs.md` and exemplified by e.g. `docs/manual-qa/NEO-31.md`. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | Matches | E2.M1 tracking row updated for **NEO-34 landed** + pointers. Done. | +| `docs/manual-qa/NEO-34.md` | Matches | Standard header table and numbered sections aligned with NEO-31 style. Done. | ## Blocking issues @@ -31,14 +31,14 @@ The branch adds fail-fast loading of `content/skills/*.json` at server startup: ## Suggestions -1. After merge (or in this PR): update **`documentation_and_implementation_alignment.md`** E2.M1 snapshot to include NEO-34 server load and pointers to `Game/Skills/` + README, so the tracking table matches reality. -2. Either add a short commented **`Content`** subsection to **`server/NeonSprawl.Server/appsettings.json`** as the plan described, or remove/adjust that row in **`NEO-34-implementation-plan.md`** so the plan matches what shipped. -3. Expand **`docs/manual-qa/NEO-34.md`** with the standard header table (Key, Title, Linear, Plan, Branch) and numbered sections for consistency with other story QA files. +1. ~~After merge (or in this PR): update **`documentation_and_implementation_alignment.md`** E2.M1 snapshot to include NEO-34 server load and pointers to `Game/Skills/` + README, so the tracking table matches reality.~~ Done. +2. ~~Either add a short commented **`Content`** subsection to **`server/NeonSprawl.Server/appsettings.json`** as the plan described, or remove/adjust that row in **`NEO-34-implementation-plan.md`** so the plan matches what shipped.~~ Done (placeholder `Content` keys in `appsettings.json` + plan row aligned). +3. ~~Expand **`docs/manual-qa/NEO-34.md`** with the standard header table (Key, Title, Linear, Plan, Branch) and numbered sections for consistency with other story QA files.~~ Done. ## Nits -- **`bruno/neon-sprawl-server/core/Health.bru`**: Added Bruno `tests` blocks for status and `service` identity. Useful, but not called out in the NEO-34 plan; consider mentioning in the PR description or splitting if you want a strictly single-concern PR. -- **`SkillDefinitionCatalogLoaderTests.cs`**: `Host_ShouldResolveCatalogFromDi_WhenStartupSucceeds` puts `GetRequiredService()` under **Act** alongside `GetAsync("/health")`; strict AAA would treat DI resolution as **Assert** only (style only). +- **`bruno/neon-sprawl-server/core/Health.bru`**: Added Bruno `tests` blocks for status and `service` identity. Useful, but not called out in the NEO-34 plan; consider mentioning in the PR description or splitting if you want a strictly single-concern PR. *(Unchanged — optional PR hygiene.)* +- ~~**`SkillDefinitionCatalogLoaderTests.cs`**: `Host_ShouldResolveCatalogFromDi_WhenStartupSucceeds` puts `GetRequiredService()` under **Act** alongside `GetAsync("/health")`; strict AAA would treat DI resolution as **Assert** only (style only).~~ Done. ## Verification diff --git a/server/NeonSprawl.Server.Tests/Game/Skills/SkillDefinitionCatalogLoaderTests.cs b/server/NeonSprawl.Server.Tests/Game/Skills/SkillDefinitionCatalogLoaderTests.cs index 2deff89..2217170 100644 --- a/server/NeonSprawl.Server.Tests/Game/Skills/SkillDefinitionCatalogLoaderTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Skills/SkillDefinitionCatalogLoaderTests.cs @@ -173,9 +173,9 @@ public class SkillDefinitionCatalogLoaderTests using var client = factory.CreateClient(); // Act var response = await client.GetAsync("/health"); - var catalog = factory.Services.GetRequiredService(); // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var catalog = factory.Services.GetRequiredService(); Assert.Equal(3, catalog.DistinctSkillCount); } diff --git a/server/NeonSprawl.Server/appsettings.json b/server/NeonSprawl.Server/appsettings.json index 18e0bca..394a263 100644 --- a/server/NeonSprawl.Server/appsettings.json +++ b/server/NeonSprawl.Server/appsettings.json @@ -6,6 +6,10 @@ } }, "AllowedHosts": "*", + "Content": { + "SkillsDirectory": "", + "SkillDefSchemaPath": "" + }, "Game": { "DevPlayerId": "dev-local-1", "DefaultPosition": {