From 82c45941ae7ad449ac811a6f7d65f3c294e50003 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 22 Jun 2026 20:36:47 -0400 Subject: [PATCH 01/18] NEO-146: add E7M4-03 contract instance and outcome stores kickoff plan --- docs/plans/NEO-146-implementation-plan.md | 228 ++++++++++++++++++++++ 1 file changed, 228 insertions(+) create mode 100644 docs/plans/NEO-146-implementation-plan.md diff --git a/docs/plans/NEO-146-implementation-plan.md b/docs/plans/NEO-146-implementation-plan.md new file mode 100644 index 0000000..40caa28 --- /dev/null +++ b/docs/plans/NEO-146-implementation-plan.md @@ -0,0 +1,228 @@ +# NEO-146 — E7M4-03: Contract instance + outcome stores + +**Linear:** [NEO-146](https://linear.app/neon-sprawl/issue/NEO-146) +**Branch:** `NEO-146-e7m4-03-contract-instance-outcome-stores` +**Backlog:** [E7M4-pre-production-backlog.md](E7M4-pre-production-backlog.md) — **E7M4-03** +**Module:** [E7_M4_ContractMissionGenerator.md](../decomposition/modules/E7_M4_ContractMissionGenerator.md) +**Pattern:** [NEO-116-implementation-plan.md](NEO-116-implementation-plan.md) (in-memory + Postgres when configured); [NEO-135-implementation-plan.md](NEO-135-implementation-plan.md) (dual-store split); [NEO-126-implementation-plan.md](NEO-126-implementation-plan.md) (grant snapshot on audit row) +**Precursor:** [NEO-145](https://linear.app/neon-sprawl/issue/NEO-145) **`Done`** — `IContractTemplateRegistry`, fail-fast contract catalog load (**on `main`**) +**Blocks:** [NEO-147](https://linear.app/neon-sprawl/issue/NEO-147) (E7M4-04 `ContractGeneratorOperations.TryIssue`) +**Client counterpart:** none (infrastructure-only); playable loop is **NEO-154** (E7M4-11) + +## Goal + +Durable per-player **`ContractInstance`** rows (active/completed) and append-only **`ContractOutcome`** audit records. Stores expose mutation primitives for issue/complete flows; generator, payout apply, and HTTP land in later stories. + +## Kickoff clarifications + +**No blocking decisions needed.** Linear AC, [E7M4-03 backlog scope](E7M4-pre-production-backlog.md#e7m4-03--contract-instance--outcome-stores), [E7M4 kickoff decisions table](E7M4-pre-production-backlog.md#kickoff-decisions-decomposition-defaults), and NEO-116/NEO-135/NEO-126 precedents settle: + +| Topic | Decision | Evidence | +|-------|----------|----------| +| Postgres persistence | **Include when configured** — in-memory fallback otherwise | Backlog “NEO-116-style split”; NEO-116/NEO-135 adopted same | +| Store split | **`IContractInstanceStore`** + **`IContractOutcomeStore`** | Backlog in-scope list; snapshot vs append-only audit | +| Store API shape | **Mutation methods on instance store** — `TryCreateActive`, `TryMarkComplete`; outcome store **`TryAppend`** + query | NEO-116 quest store precedent; backlog unit tests “issue, complete, idempotent complete replay” | +| One active per player | **Enforce in `TryCreateActive`** — deny when player already has `active` row | Linear AC + kickoff “One active contract per player” | +| `expired` status | **Enum value only** — no transition API in E7M4-03 | Backlog instance row lists status; no expiry story in Slice 4 backlog | +| Missing instance | **Implicit absence** — `TryGet` returns `false`; no `not_started` row | NEO-116 missing-row pattern | +| Player write gate | **`CanWritePlayer`** on instance store (dev bucket / Postgres `player_position`) | NEO-116 quest store precedent | +| Template validation | **Store persists `templateId` string only** — registry checks deferred to E7M4-04 generator | NEO-116 does not validate quest ids at store boundary | +| Outcome grant snapshot | **Reuse `RewardItemGrantApplied` / `RewardSkillXpGrantApplied` / `RewardReputationGrantApplied`** from `Game/Rewards/` | Contract completion bundle same grant kinds (E7M4-05); avoids duplicate DTOs | +| Idempotency key on outcome | **`{playerId}:contract_complete:{contractInstanceId}`** stored on outcome row | E7M4 kickoff decisions table | +| Generator / payout / HTTP | **Out of scope** | Backlog E7M4-03; NEO-147+ | +| DI registration | **`AddContractInstanceStores`** extension separate from catalog | NEO-116 `AddPlayerQuestStateStore` precedent | + +## Scope and out-of-scope + +**In scope (from Linear + backlog):** + +- **`ContractInstanceState`** snapshot + **`ContractInstanceStatus`** (`Active`, `Completed`, `Expired`). +- **`IContractInstanceStore`**, **`IContractOutcomeStore`** — in-memory + Postgres when `ConnectionStrings:NeonSprawl` is set. +- Instance fields: `contractInstanceId`, `templateId`, `playerId`, `status`, `seedBucket`, `issuedAt`, optional `completedAt`. +- Outcome fields: instance id, delivery idempotency key, grant snapshot summary, `recordedAt`, row id. +- **`ContractInstanceIds`** — player/instance id normalization + composite keys. +- **`V011__contract_instance.sql`**, **`V012__contract_outcome.sql`** + bootstrap helpers when Postgres configured. +- Unit tests (AAA): create active (issue primitive), deny second active, complete, idempotent complete replay, outcome append-only + duplicate deny. +- Postgres persistence integration tests (`RequirePostgresFact`) for both stores. +- `server/README.md` contract instance + outcome store sections. +- DI wired from **`Program.cs`**; in-memory override in **`InMemoryWebApplicationFactory`**. + +**Out of scope (from Linear + backlog):** + +- **`ContractGeneratorOperations.TryIssue`** (NEO-147 / E7M4-04). +- **`RewardRouterOperations.TryDeliverContractCompletion`** (NEO-148 / E7M4-05). +- Encounter completion wiring (NEO-149 / E7M4-06). +- Economy validation at issue (NEO-150 / E7M4-07). +- HTTP issue/GET (NEO-151 / E7M4-08), telemetry hooks (NEO-152), Godot (NEO-153+). + +**Client counterpart:** none (infrastructure-only). + +## Acceptance criteria checklist + +- [ ] At most one **`active`** instance per player (prototype policy). +- [ ] Completed instances immutable; outcome append-only. +- [ ] `dotnet test` covers store gates. + +## Technical approach + +### 1. Types (`Game/Contracts/`) + +| Type | Role | +|------|------| +| `ContractInstanceStatus` | `Active`, `Completed`, `Expired` (no expiry transition in E7M4-03) | +| `ContractInstanceState` | Immutable snapshot: `ContractInstanceId`, `TemplateId`, `PlayerId`, `Status`, `SeedBucket`, `IssuedAt`, `CompletedAt?` | +| `ContractInstanceIds` | `NormalizePlayerId`, `NormalizeContractInstanceId`, `MakeInstanceKey`, `MakeActivePlayerKey` | +| `ContractInstanceReasonCodes` | `player_not_writable`, `active_contract_exists`, `instance_not_found`, `instance_not_active`, `invalid_ids` | +| `ContractOutcomeRow` | `Id`, `ContractInstanceId`, `PlayerId`, `IdempotencyKey`, grant snapshots (items/skill XP/rep), `RecordedAt` | +| `ContractOutcomeIds` | `MakeIdempotencyKey(playerId, contractInstanceId)` → `{p}:contract_complete:{i}` | + +Grant snapshot fields on **`ContractOutcomeRow`** use existing reward types: + +- `IReadOnlyList GrantedItems` +- `IReadOnlyList GrantedSkillXp` +- `IReadOnlyList GrantedReputation` + +### 2. `IContractInstanceStore` + +Injected by **`ContractGeneratorOperations`** (NEO-147) and **`ContractCompletionOperations`** (NEO-149); not HTTP directly. + +- **`bool CanWritePlayer(string playerId)`** — dev bucket or Postgres `player_position` row exists. +- **`bool TryGet(string playerId, string contractInstanceId, out ContractInstanceState snapshot)`** — missing ⇒ `false`. +- **`bool TryGetActiveForPlayer(string playerId, out ContractInstanceState snapshot)`** — at most one active row per player; missing active ⇒ `false`. +- **`bool TryCreateActive(string playerId, string contractInstanceId, string templateId, string seedBucket, DateTimeOffset issuedAt, out ContractInstanceState snapshot)`** — first active for player returns `true`; denies when player not writable, invalid ids, duplicate instance id, or **`active_contract_exists`** when another active row present. +- **`bool TryMarkComplete(string playerId, string contractInstanceId, DateTimeOffset completedAt, out ContractInstanceState snapshot)`** — requires active row; first complete `true`; replay `false` preserving first `CompletedAt`; completed rows cannot regress. +- **`bool TryClearInstance(string playerId, string contractInstanceId)`** — dev fixture only; idempotent when absent (Bruno/test reset parity with quest store `TryClearProgress`). + +### 3. `IContractOutcomeStore` + +- **`bool TryAppend(ContractOutcomeRow row)`** — append-only; `false` on duplicate `Id` or duplicate `IdempotencyKey` or invalid row. +- **`bool TryGetByIdempotencyKey(string idempotencyKey, out ContractOutcomeRow row)`** — audit/idempotency read for tests and E7M4-05 router replay. +- **`IReadOnlyList GetOutcomesForInstance(string contractInstanceId, int? limit = null)`** — ordered by `RecordedAt` then `Id` ascending. +- **`bool TryClearForInstance(string contractInstanceId)`** — dev fixture only. + +No update/delete APIs in prototype. + +### 4. In-memory implementations + +Mirror NEO-116 / NEO-135: + +- **`InMemoryContractInstanceStore`** — `ConcurrentDictionary` by instance key; secondary index `playerId → active instance id`; dev player seeded from `GamePositionOptions`; per-key locks for mutations. +- **`InMemoryContractOutcomeStore`** — append-only by row id + idempotency key index; thread-safe. + +### 5. Postgres implementations + +Mirror NEO-116 quest store: + +- **`PostgresContractInstanceBootstrap`** + **`V011__contract_instance.sql`** + - Table `contract_instance (contract_instance_id TEXT PK, player_id FK → player_position, template_id, status, seed_bucket, issued_at, completed_at NULL)` + - Partial unique index on `(player_id) WHERE status = 'active'` — DB-enforced one-active policy +- **`PostgresContractOutcomeBootstrap`** + **`V012__contract_outcome.sql`** + - Table `contract_outcome (id TEXT PK, contract_instance_id FK, player_id FK, idempotency_key UNIQUE, granted_items JSONB, granted_skill_xp JSONB, granted_reputation JSONB, recorded_at)` + - Index on `(contract_instance_id, recorded_at)` +- **`PostgresContractInstanceStore`**, **`PostgresContractOutcomeStore`** — `EnsureSchema` on first use; instance store checks writable player before insert/update. + +JSON columns store grant snapshots as compact arrays (same shape as in-memory lists); no behavior change vs in-memory. + +### 6. DI and `Program.cs` + +- **`ContractInstanceServiceCollectionExtensions.AddContractInstanceStores`** — Postgres implementations when connection string set; otherwise in-memory pair. +- Register **after** `AddContractTemplateCatalog` (catalog unrelated; stores are runtime state). +- **`InMemoryWebApplicationFactory`:** remove/replace Postgres registrations for both store interfaces; register in-memory implementations in `ConfigureTestServices` loop (same pattern as quest/faction stores). + +### 7. Tests + +**Instance store (`InMemoryContractInstanceStoreTests`):** + +- Create active (happy path) — all fields persisted. +- Deny second active for same player (`active_contract_exists`). +- Complete active instance; snapshot status `Completed` + `CompletedAt`. +- Idempotent complete replay — second `TryMarkComplete` returns `false`, `CompletedAt` unchanged. +- Completed instance cannot accept new active without dev clear (optional: completed player can issue again after complete — **new instance id** allowed when no active row). +- Deny create when player not writable / empty ids. +- Host DI: `Host_ShouldResolveContractInstanceStoresFromDi`. + +**Outcome store (`InMemoryContractOutcomeStoreTests`):** + +- Append first outcome; query by instance + idempotency key. +- Duplicate row id or idempotency key denies append. +- Append-only — no mutation APIs. + +**Postgres (`ContractInstancePersistenceIntegrationTests`, `ContractOutcomePersistenceIntegrationTests`):** + +- `RequirePostgresFact`: write on factory A, read on factory B for instance create/complete and outcome append. + +### 8. Docs + +- **`server/README.md`** — contract instance store + contract outcome store sections (config, one-active policy, idempotency key shape, load order note: stores are runtime, not content catalog). +- Optional one-line E7_M4 module store note when implementation lands. + +### 9. Implementation order + +1. Types, ids, reason codes. +2. In-memory stores + interfaces. +3. Postgres migrations + stores. +4. DI extension, `Program.cs`, factory overrides. +5. Unit tests → Postgres integration tests → host DI smoke. +6. `server/README.md`. +7. Run `dotnet test`. + +## Files to add + +| Path | Purpose | +|------|---------| +| `docs/plans/NEO-146-implementation-plan.md` | This plan. | +| `server/NeonSprawl.Server/Game/Contracts/ContractInstanceStatus.cs` | Instance lifecycle enum | +| `server/NeonSprawl.Server/Game/Contracts/ContractInstanceState.cs` | Immutable instance snapshot | +| `server/NeonSprawl.Server/Game/Contracts/ContractInstanceIds.cs` | Id normalization + composite keys | +| `server/NeonSprawl.Server/Game/Contracts/ContractInstanceReasonCodes.cs` | Stable deny reason strings | +| `server/NeonSprawl.Server/Game/Contracts/IContractInstanceStore.cs` | Instance store contract | +| `server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs` | Thread-safe in-memory instance store | +| `server/NeonSprawl.Server/Game/Contracts/ContractOutcomeRow.cs` | Append-only outcome audit row | +| `server/NeonSprawl.Server/Game/Contracts/ContractOutcomeIds.cs` | Idempotency key helper | +| `server/NeonSprawl.Server/Game/Contracts/IContractOutcomeStore.cs` | Outcome store contract | +| `server/NeonSprawl.Server/Game/Contracts/InMemoryContractOutcomeStore.cs` | Thread-safe in-memory outcome store | +| `server/NeonSprawl.Server/Game/Contracts/PostgresContractInstanceBootstrap.cs` | Schema ensure for instance table | +| `server/NeonSprawl.Server/Game/Contracts/PostgresContractInstanceStore.cs` | Postgres instance store | +| `server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeBootstrap.cs` | Schema ensure for outcome table | +| `server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs` | Postgres outcome store | +| `server/NeonSprawl.Server/Game/Contracts/ContractInstanceServiceCollectionExtensions.cs` | DI registration | +| `server/db/migrations/V011__contract_instance.sql` | Postgres instance migration | +| `server/db/migrations/V012__contract_outcome.sql` | Postgres outcome migration | +| `server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs` | Instance store AAA tests | +| `server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs` | Outcome store AAA tests | +| `server/NeonSprawl.Server.Tests/Game/Contracts/ContractInstancePersistenceIntegrationTests.cs` | Postgres instance persistence | +| `server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs` | Postgres outcome persistence | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Program.cs` | Register `AddContractInstanceStores` after contract catalog | +| `server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs` | Force in-memory contract stores in tests (override Postgres registrations) | +| `server/README.md` | Document contract instance + outcome stores | + +## Tests + +| Layer | What | +|-------|------| +| `InMemoryContractInstanceStoreTests` | Create active, one-active deny, complete, idempotent complete, immutability, writable gate, empty-id deny | +| `InMemoryContractOutcomeStoreTests` | Append, duplicate deny, query by instance/idempotency key, append-only | +| `ContractInstancePersistenceIntegrationTests` | Postgres round-trip create + complete (`RequirePostgresFact`) | +| `ContractOutcomePersistenceIntegrationTests` | Postgres round-trip append + read (`RequirePostgresFact`) | +| Host boot | Resolve `IContractInstanceStore` + `IContractOutcomeStore` from DI via factory | +| CI | Existing `dotnet test`; no Python/content changes expected | + +Manual Godot QA: **none** (server infrastructure; capstone is NEO-154). + +## Open questions / risks + +| Question / risk | Agent recommendation | Status | +|-----------------|----------------------|--------| +| Postgres partial unique index vs app-only one-active check | **Both** — enforce in store logic (fast deny reason) + partial unique index for race safety | `adopted` | +| Grant snapshot on outcome before E7M4-05 router | Store accepts caller-supplied snapshot on append; E7M4-06 orchestrator fills from router result | `adopted` | +| `Expired` status unused in Slice 4 | Keep enum for forward compatibility; no API until a future expiry story | `adopted` | +| Dev `TryClear*` on Postgres | Implement on in-memory only first; add Postgres clear if Bruno fixture story needs it (mirror quest `TryClearProgress` on both when needed) | `deferred` | + +## Client counterpart + +None for E7M4-03. Client contract HUD: **NEO-153** / capstone **NEO-154**. From ae641a4dd0d004def60d9291baea982e96ee24db Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 24 Jun 2026 20:27:32 -0400 Subject: [PATCH 02/18] NEO-146: add contract instance and outcome stores with Postgres split Durable per-player contract instances (one active) and append-only outcome audit rows, mirroring NEO-116/135 store patterns with V011/V012 migrations, AAA tests, README docs, and Bruno health smoke. --- ...th after contract instance stores load.bru | 25 ++ .../contract-stores/folder.bru | 3 + .../modules/E7_M4_ContractMissionGenerator.md | 2 + docs/plans/NEO-146-implementation-plan.md | 14 +- ...ractInstancePersistenceIntegrationTests.cs | 132 ++++++ ...tractOutcomePersistenceIntegrationTests.cs | 120 ++++++ .../InMemoryContractInstanceStoreTests.cs | 231 +++++++++++ .../InMemoryContractOutcomeStoreTests.cs | 87 ++++ .../InMemoryWebApplicationFactory.cs | 4 + .../Game/Contracts/ContractInstanceIds.cs | 43 ++ .../Contracts/ContractInstanceReasonCodes.cs | 15 + ...ractInstanceServiceCollectionExtensions.cs | 25 ++ .../Game/Contracts/ContractInstanceState.cs | 26 ++ .../Game/Contracts/ContractInstanceStatus.cs | 9 + .../Game/Contracts/ContractOutcomeIds.cs | 18 + .../Game/Contracts/ContractOutcomeRow.cs | 14 + .../Game/Contracts/IContractInstanceStore.cs | 39 ++ .../Game/Contracts/IContractOutcomeStore.cs | 16 + .../InMemoryContractInstanceStore.cs | 250 ++++++++++++ .../Contracts/InMemoryContractOutcomeStore.cs | 133 +++++++ .../PostgresContractInstanceBootstrap.cs | 39 ++ .../PostgresContractInstanceStore.cs | 375 ++++++++++++++++++ .../PostgresContractOutcomeBootstrap.cs | 39 ++ .../Contracts/PostgresContractOutcomeStore.cs | 203 ++++++++++ server/NeonSprawl.Server/Program.cs | 1 + server/README.md | 30 ++ .../db/migrations/V011__contract_instance.sql | 17 + .../db/migrations/V012__contract_outcome.sql | 17 + 28 files changed, 1924 insertions(+), 3 deletions(-) create mode 100644 bruno/neon-sprawl-server/contract-stores/Health after contract instance stores load.bru create mode 100644 bruno/neon-sprawl-server/contract-stores/folder.bru create mode 100644 server/NeonSprawl.Server.Tests/Game/Contracts/ContractInstancePersistenceIntegrationTests.cs create mode 100644 server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs create mode 100644 server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs create mode 100644 server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs create mode 100644 server/NeonSprawl.Server/Game/Contracts/ContractInstanceIds.cs create mode 100644 server/NeonSprawl.Server/Game/Contracts/ContractInstanceReasonCodes.cs create mode 100644 server/NeonSprawl.Server/Game/Contracts/ContractInstanceServiceCollectionExtensions.cs create mode 100644 server/NeonSprawl.Server/Game/Contracts/ContractInstanceState.cs create mode 100644 server/NeonSprawl.Server/Game/Contracts/ContractInstanceStatus.cs create mode 100644 server/NeonSprawl.Server/Game/Contracts/ContractOutcomeIds.cs create mode 100644 server/NeonSprawl.Server/Game/Contracts/ContractOutcomeRow.cs create mode 100644 server/NeonSprawl.Server/Game/Contracts/IContractInstanceStore.cs create mode 100644 server/NeonSprawl.Server/Game/Contracts/IContractOutcomeStore.cs create mode 100644 server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs create mode 100644 server/NeonSprawl.Server/Game/Contracts/InMemoryContractOutcomeStore.cs create mode 100644 server/NeonSprawl.Server/Game/Contracts/PostgresContractInstanceBootstrap.cs create mode 100644 server/NeonSprawl.Server/Game/Contracts/PostgresContractInstanceStore.cs create mode 100644 server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeBootstrap.cs create mode 100644 server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs create mode 100644 server/db/migrations/V011__contract_instance.sql create mode 100644 server/db/migrations/V012__contract_outcome.sql diff --git a/bruno/neon-sprawl-server/contract-stores/Health after contract instance stores load.bru b/bruno/neon-sprawl-server/contract-stores/Health after contract instance stores load.bru new file mode 100644 index 0000000..54f94c5 --- /dev/null +++ b/bruno/neon-sprawl-server/contract-stores/Health after contract instance stores load.bru @@ -0,0 +1,25 @@ +meta { + name: GET health (contract instance stores NEO-146) + type: http + seq: 1 +} + +get { + url: {{baseUrl}}/health + body: none + auth: none +} + +docs { + NEO-146 registers IContractInstanceStore and IContractOutcomeStore (in-memory or Postgres when configured). No contract HTTP API in this story — use this request to confirm the host started after store DI wiring. +} + +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/bruno/neon-sprawl-server/contract-stores/folder.bru b/bruno/neon-sprawl-server/contract-stores/folder.bru new file mode 100644 index 0000000..550f2d4 --- /dev/null +++ b/bruno/neon-sprawl-server/contract-stores/folder.bru @@ -0,0 +1,3 @@ +meta { + name: contract-stores +} diff --git a/docs/decomposition/modules/E7_M4_ContractMissionGenerator.md b/docs/decomposition/modules/E7_M4_ContractMissionGenerator.md index aafbd40..61e8ff4 100644 --- a/docs/decomposition/modules/E7_M4_ContractMissionGenerator.md +++ b/docs/decomposition/modules/E7_M4_ContractMissionGenerator.md @@ -55,6 +55,8 @@ Epic 7 **Slice 4** — `contract_issued`, `contract_complete`, reward anomalies. **Server load (NEO-145):** host fail-fast load of `content/contracts/*_contract_templates.json` via `ContractTemplateCatalogLoader` + **`IContractTemplateRegistry`** — same E7M4 gates as CI at startup; [server README — Contract template catalog](../../../server/README.md#contract-template-catalog-contentcontracts-neo-145). +**Runtime stores (NEO-146):** per-player **`IContractInstanceStore`** (one active instance) + append-only **`IContractOutcomeStore`**; [server README — Contract instance store](../../../server/README.md#contract-instance-store-neo-146). + ## Source anchors - Master plan: [`neon_sprawl_vision.plan.md`](../../../neon_sprawl_vision.plan.md) — Epic 7. diff --git a/docs/plans/NEO-146-implementation-plan.md b/docs/plans/NEO-146-implementation-plan.md index 40caa28..8bf5213 100644 --- a/docs/plans/NEO-146-implementation-plan.md +++ b/docs/plans/NEO-146-implementation-plan.md @@ -59,9 +59,17 @@ Durable per-player **`ContractInstance`** rows (active/completed) and append-onl ## Acceptance criteria checklist -- [ ] At most one **`active`** instance per player (prototype policy). -- [ ] Completed instances immutable; outcome append-only. -- [ ] `dotnet test` covers store gates. +- [x] At most one **`active`** instance per player (prototype policy). +- [x] Completed instances immutable; outcome append-only. +- [x] `dotnet test` covers store gates. + +## Implementation reconciliation (shipped) + +- **Types:** `ContractInstanceStatus`, `ContractInstanceState`, `ContractInstanceIds`, `ContractInstanceReasonCodes`, `ContractOutcomeRow`, `ContractOutcomeIds`. +- **Stores:** `IContractInstanceStore` / `IContractOutcomeStore`; in-memory + Postgres (`V011`, `V012`) when connection string set; partial unique index + app-level one-active enforcement. +- **DI:** `AddContractInstanceStores` in `Program.cs`; in-memory override in `InMemoryWebApplicationFactory`. +- **Tests:** `InMemoryContractInstanceStoreTests`, `InMemoryContractOutcomeStoreTests`, Postgres persistence integration tests; host DI smoke (`862` tests green). +- **Docs:** `server/README.md` contract instance + outcome store sections. ## Technical approach diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractInstancePersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractInstancePersistenceIntegrationTests.cs new file mode 100644 index 0000000..4fa4f9e --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractInstancePersistenceIntegrationTests.cs @@ -0,0 +1,132 @@ +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using NeonSprawl.Server.Game.Contracts; +using NeonSprawl.Server.Game.PositionState; +using NeonSprawl.Server.Tests.Game.PositionState; +using Npgsql; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.Contracts; + +[Collection("Postgres integration")] +public sealed class ContractInstancePersistenceIntegrationTests(PostgresIntegrationHarness harness) +{ + private PostgresWebApplicationFactory Factory => harness.Factory; + + private const string PlayerId = "dev-local-1"; + private const string InstanceId = "prototype_contract_instance_pg_001"; + private const string TemplateId = "prototype_contract_clear_combat_pocket"; + private const string SeedBucket = "2026-06-22"; + private static readonly DateTimeOffset IssuedAt = new(2026, 6, 22, 10, 0, 0, TimeSpan.Zero); + private static readonly DateTimeOffset CompletedAt = new(2026, 6, 22, 14, 0, 0, TimeSpan.Zero); + + [RequirePostgresFact] + public async Task TryCreateActive_ShouldReturnOneTrueAndRestFalse_WhenCalledConcurrently() + { + // Arrange + await ResetContractInstanceTableAsync(); + const int concurrentCalls = 8; + + // Act + var results = await Task.WhenAll( + Enumerable.Range(0, concurrentCalls).Select(_ => Task.Run(() => + { + using var scope = Factory.Services.CreateScope(); + var store = scope.ServiceProvider.GetRequiredService(); + return store.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out ContractInstanceState _); + }))); + + // Assert + Assert.Equal(1, results.Count(static r => r)); + Assert.Equal(concurrentCalls - 1, results.Count(static r => !r)); + Assert.True( + Factory.Services.CreateScope().ServiceProvider + .GetRequiredService() + .TryGet(PlayerId, InstanceId, out var snapshot)); + Assert.Equal(ContractInstanceStatus.Active, snapshot.Status); + Assert.Equal(TemplateId, snapshot.TemplateId); + } + + [RequirePostgresFact] + public async Task CreateComplete_ShouldPersistAcrossNewFactory() + { + // Arrange + await ResetContractInstanceTableAsync(); + + // Act — write through first host + using (var firstScope = Factory.Services.CreateScope()) + { + var store = firstScope.ServiceProvider.GetRequiredService(); + Assert.True(store.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _)); + Assert.True(store.TryMarkComplete(PlayerId, InstanceId, CompletedAt, out _)); + } + + ContractInstanceState readBack; + await using (var secondFactory = new PostgresWebApplicationFactory()) + { + using var secondScope = secondFactory.Services.CreateScope(); + var store = secondScope.ServiceProvider.GetRequiredService(); + readBack = store.TryGet(PlayerId, InstanceId, out var snapshot) + ? snapshot + : null!; + } + + // Assert + Assert.NotNull(readBack); + Assert.Equal(ContractInstanceStatus.Completed, readBack.Status); + Assert.Equal(SeedBucket, readBack.SeedBucket); + Assert.Equal(CompletedAt, readBack.CompletedAt); + } + + private async Task ResetContractInstanceTableAsync() + { + var cs = Environment.GetEnvironmentVariable("ConnectionStrings__NeonSprawl"); + if (string.IsNullOrWhiteSpace(cs)) + { + throw new InvalidOperationException("ConnectionStrings__NeonSprawl is not set."); + } + + _ = Factory.Services; + var options = Factory.Services.GetRequiredService>().Value; + + var positionDdlPath = Path.Combine(AppContext.BaseDirectory, "db", "migrations", "V001__player_position.sql"); + var instanceDdlPath = Path.Combine(AppContext.BaseDirectory, "db", "migrations", "V011__contract_instance.sql"); + if (!File.Exists(positionDdlPath) || !File.Exists(instanceDdlPath)) + { + throw new FileNotFoundException("Test DDL for contract instance persistence not found."); + } + + var positionDdl = await File.ReadAllTextAsync(positionDdlPath); + var instanceDdl = await File.ReadAllTextAsync(instanceDdlPath); + await using var conn = new NpgsqlConnection(cs); + await conn.OpenAsync(); + await using (var applyPosition = new NpgsqlCommand(positionDdl, conn)) + { + await applyPosition.ExecuteNonQueryAsync(); + } + + await using (var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn)) + { + await truncate.ExecuteNonQueryAsync(); + } + + PostgresPositionBootstrap.SeedDevPlayer(conn, options); + + await using (var applyInstance = new NpgsqlCommand(instanceDdl, conn)) + { + await applyInstance.ExecuteNonQueryAsync(); + } + } +} diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs new file mode 100644 index 0000000..3fd1bae --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs @@ -0,0 +1,120 @@ +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using NeonSprawl.Server.Game.Contracts; +using NeonSprawl.Server.Game.PositionState; +using NeonSprawl.Server.Game.Rewards; +using NeonSprawl.Server.Tests.Game.PositionState; +using Npgsql; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.Contracts; + +[Collection("Postgres integration")] +public sealed class ContractOutcomePersistenceIntegrationTests(PostgresIntegrationHarness harness) +{ + private PostgresWebApplicationFactory Factory => harness.Factory; + + private const string PlayerId = "dev-local-1"; + private const string InstanceId = "prototype_contract_instance_pg_outcome_001"; + private const string TemplateId = "prototype_contract_clear_combat_pocket"; + private const string SeedBucket = "2026-06-22"; + private const string OutcomeRowId = "outcome-row-pg-001"; + private static readonly DateTimeOffset IssuedAt = new(2026, 6, 22, 10, 0, 0, TimeSpan.Zero); + private static readonly DateTimeOffset RecordedAt = new(2026, 6, 22, 14, 0, 0, TimeSpan.Zero); + + [RequirePostgresFact] + public async Task AppendOutcome_ShouldPersistAcrossNewFactory() + { + // Arrange + await ResetContractTablesAsync(); + var idempotencyKey = ContractOutcomeIds.MakeIdempotencyKey(PlayerId, InstanceId); + var outcomeRow = new ContractOutcomeRow( + OutcomeRowId, + InstanceId, + PlayerId, + idempotencyKey, + [new RewardItemGrantApplied("scrap_metal_bulk", 5)], + [new RewardSkillXpGrantApplied("salvage", 15)], + [], + RecordedAt); + + // Act — write through first host + using (var firstScope = Factory.Services.CreateScope()) + { + var instanceStore = firstScope.ServiceProvider.GetRequiredService(); + var outcomeStore = firstScope.ServiceProvider.GetRequiredService(); + Assert.True(instanceStore.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _)); + Assert.True(outcomeStore.TryAppend(outcomeRow)); + } + + ContractOutcomeRow readBack; + await using (var secondFactory = new PostgresWebApplicationFactory()) + { + using var secondScope = secondFactory.Services.CreateScope(); + var outcomeStore = secondScope.ServiceProvider.GetRequiredService(); + readBack = outcomeStore.TryGetByIdempotencyKey(idempotencyKey, out var row) + ? row + : null!; + } + + // Assert + Assert.NotNull(readBack); + Assert.Equal(OutcomeRowId, readBack.Id); + Assert.Equal(InstanceId, readBack.ContractInstanceId); + Assert.Equal(5, readBack.GrantedItems[0].Quantity); + Assert.Equal(15, readBack.GrantedSkillXp[0].Amount); + } + + private async Task ResetContractTablesAsync() + { + var cs = Environment.GetEnvironmentVariable("ConnectionStrings__NeonSprawl"); + if (string.IsNullOrWhiteSpace(cs)) + { + throw new InvalidOperationException("ConnectionStrings__NeonSprawl is not set."); + } + + _ = Factory.Services; + var options = Factory.Services.GetRequiredService>().Value; + + var positionDdlPath = Path.Combine(AppContext.BaseDirectory, "db", "migrations", "V001__player_position.sql"); + var instanceDdlPath = Path.Combine(AppContext.BaseDirectory, "db", "migrations", "V011__contract_instance.sql"); + var outcomeDdlPath = Path.Combine(AppContext.BaseDirectory, "db", "migrations", "V012__contract_outcome.sql"); + if (!File.Exists(positionDdlPath) || !File.Exists(instanceDdlPath) || !File.Exists(outcomeDdlPath)) + { + throw new FileNotFoundException("Test DDL for contract outcome persistence not found."); + } + + var positionDdl = await File.ReadAllTextAsync(positionDdlPath); + var instanceDdl = await File.ReadAllTextAsync(instanceDdlPath); + var outcomeDdl = await File.ReadAllTextAsync(outcomeDdlPath); + await using var conn = new NpgsqlConnection(cs); + await conn.OpenAsync(); + await using (var applyPosition = new NpgsqlCommand(positionDdl, conn)) + { + await applyPosition.ExecuteNonQueryAsync(); + } + + await using (var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn)) + { + await truncate.ExecuteNonQueryAsync(); + } + + PostgresPositionBootstrap.SeedDevPlayer(conn, options); + + await using (var applyInstance = new NpgsqlCommand(instanceDdl, conn)) + { + await applyInstance.ExecuteNonQueryAsync(); + } + + await using (var applyOutcome = new NpgsqlCommand(outcomeDdl, conn)) + { + await applyOutcome.ExecuteNonQueryAsync(); + } + } +} diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs new file mode 100644 index 0000000..eaae754 --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs @@ -0,0 +1,231 @@ +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using NeonSprawl.Server.Game.Contracts; +using NeonSprawl.Server.Game.PositionState; +using NeonSprawl.Server.Tests; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.Contracts; + +public sealed class InMemoryContractInstanceStoreTests +{ + private const string PlayerId = "dev-local-1"; + private const string UnknownPlayerId = "unknown-player-xyz"; + private const string InstanceId = "prototype_contract_instance_001"; + private const string SecondInstanceId = "prototype_contract_instance_002"; + private const string TemplateId = "prototype_contract_clear_combat_pocket"; + private const string SeedBucket = "2026-06-22"; + private static readonly DateTimeOffset IssuedAt = new(2026, 6, 22, 10, 0, 0, TimeSpan.Zero); + private static readonly DateTimeOffset CompletedAt = new(2026, 6, 22, 12, 0, 0, TimeSpan.Zero); + + [Fact] + public void TryGet_ShouldReturnFalse_WhenRowMissing() + { + // Arrange + var store = CreateStore(); + // Act + var found = store.TryGet(PlayerId, InstanceId, out _); + // Assert + Assert.False(found); + } + + [Fact] + public void TryCreateActive_ShouldPersistAllFields() + { + // Arrange + var store = CreateStore(); + // Act + var created = store.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out var snapshot); + // Assert + Assert.True(created); + Assert.Equal(InstanceId, snapshot.ContractInstanceId); + Assert.Equal(TemplateId, snapshot.TemplateId); + Assert.Equal(PlayerId, snapshot.PlayerId); + Assert.Equal(ContractInstanceStatus.Active, snapshot.Status); + Assert.Equal(SeedBucket, snapshot.SeedBucket); + Assert.Equal(IssuedAt, snapshot.IssuedAt); + Assert.Null(snapshot.CompletedAt); + } + + [Fact] + public void TryCreateActive_ShouldDenySecondActiveForSamePlayer() + { + // Arrange + var store = CreateStore(); + Assert.True(store.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _)); + // Act + var second = store.TryCreateActive( + PlayerId, + SecondInstanceId, + TemplateId, + SeedBucket, + IssuedAt.AddHours(1), + out var snapshot); + // Assert + Assert.False(second); + Assert.Equal(InstanceId, snapshot.ContractInstanceId); + Assert.Equal(ContractInstanceStatus.Active, snapshot.Status); + Assert.True(store.TryGetActiveForPlayer(PlayerId, out var active)); + Assert.Equal(InstanceId, active.ContractInstanceId); + } + + [Fact] + public void TryMarkComplete_ShouldSetCompletedStatusAndTimestamp() + { + // Arrange + var store = CreateStore(); + Assert.True(store.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _)); + // Act + var completed = store.TryMarkComplete(PlayerId, InstanceId, CompletedAt, out var snapshot); + // Assert + Assert.True(completed); + Assert.Equal(ContractInstanceStatus.Completed, snapshot.Status); + Assert.Equal(CompletedAt, snapshot.CompletedAt); + Assert.False(store.TryGetActiveForPlayer(PlayerId, out _)); + } + + [Fact] + public void TryMarkComplete_ShouldBeIdempotent() + { + // Arrange + var store = CreateStore(); + Assert.True(store.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _)); + // Act + var first = store.TryMarkComplete(PlayerId, InstanceId, CompletedAt, out var firstSnapshot); + var second = store.TryMarkComplete( + PlayerId, + InstanceId, + CompletedAt.AddHours(1), + out var secondSnapshot); + // Assert + Assert.True(first); + Assert.False(second); + Assert.Equal(ContractInstanceStatus.Completed, secondSnapshot.Status); + Assert.Equal(CompletedAt, secondSnapshot.CompletedAt); + Assert.Equal(firstSnapshot.TemplateId, secondSnapshot.TemplateId); + } + + [Fact] + public void TryCreateActive_ShouldAllowNewInstanceAfterComplete() + { + // Arrange + var store = CreateStore(); + Assert.True(store.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _)); + Assert.True(store.TryMarkComplete(PlayerId, InstanceId, CompletedAt, out _)); + // Act + var reissued = store.TryCreateActive( + PlayerId, + SecondInstanceId, + TemplateId, + SeedBucket, + IssuedAt.AddDays(1), + out var snapshot); + // Assert + Assert.True(reissued); + Assert.Equal(SecondInstanceId, snapshot.ContractInstanceId); + Assert.Equal(ContractInstanceStatus.Active, snapshot.Status); + } + + [Fact] + public void TryCreateActive_ShouldReturnFalse_ForUnknownPlayer() + { + // Arrange + var store = CreateStore(); + // Act + var created = store.TryCreateActive( + UnknownPlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _); + // Assert + Assert.False(created); + } + + [Theory] + [InlineData("", InstanceId, TemplateId, SeedBucket)] + [InlineData(" ", InstanceId, TemplateId, SeedBucket)] + [InlineData(PlayerId, "", TemplateId, SeedBucket)] + [InlineData(PlayerId, " ", TemplateId, SeedBucket)] + [InlineData(PlayerId, InstanceId, "", SeedBucket)] + [InlineData(PlayerId, InstanceId, TemplateId, "")] + public void TryCreateActive_ShouldReturnFalse_WhenRequiredIdsEmpty( + string playerId, + string instanceId, + string templateId, + string seedBucket) + { + // Arrange + var store = CreateStore(); + // Act + var created = store.TryCreateActive( + playerId, + instanceId, + templateId, + seedBucket, + IssuedAt, + out _); + // Assert + Assert.False(created); + } + + [Fact] + public async Task Host_ShouldResolveContractInstanceStoresFromDi() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + using var scope = factory.Services.CreateScope(); + var instanceStore = scope.ServiceProvider.GetRequiredService(); + var outcomeStore = scope.ServiceProvider.GetRequiredService(); + // Act + var created = instanceStore.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out var snapshot); + // Assert + Assert.IsType(instanceStore); + Assert.IsType(outcomeStore); + Assert.True(created); + Assert.Equal(TemplateId, snapshot.TemplateId); + } + + private static InMemoryContractInstanceStore CreateStore() + { + var options = Options.Create(new GamePositionOptions { DevPlayerId = PlayerId }); + return new InMemoryContractInstanceStore(options); + } +} diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs new file mode 100644 index 0000000..73f0787 --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs @@ -0,0 +1,87 @@ +using NeonSprawl.Server.Game.Contracts; +using NeonSprawl.Server.Game.Rewards; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.Contracts; + +public sealed class InMemoryContractOutcomeStoreTests +{ + private const string PlayerId = "dev-local-1"; + private const string InstanceId = "prototype_contract_instance_001"; + private const string OutcomeRowId = "outcome-row-001"; + private static readonly DateTimeOffset RecordedAt = new(2026, 6, 22, 12, 0, 0, TimeSpan.Zero); + + [Fact] + public void TryAppend_ShouldPersistAndQueryByInstanceAndIdempotencyKey() + { + // Arrange + var store = new InMemoryContractOutcomeStore(); + var row = CreatePrototypeOutcome(); + // Act + var appended = store.TryAppend(row); + var foundByKey = store.TryGetByIdempotencyKey(row.IdempotencyKey, out var byKey); + var listed = store.GetOutcomesForInstance(InstanceId); + // Assert + Assert.True(appended); + Assert.True(foundByKey); + Assert.Equal(row.Id, byKey.Id); + Assert.Single(listed); + Assert.Equal("scrap_metal_bulk", listed[0].GrantedItems[0].ItemId); + Assert.Equal(5, listed[0].GrantedItems[0].Quantity); + Assert.Equal("salvage", listed[0].GrantedSkillXp[0].SkillId); + Assert.Equal(15, listed[0].GrantedSkillXp[0].Amount); + } + + [Fact] + public void TryAppend_ShouldDenyDuplicateRowId() + { + // Arrange + var store = new InMemoryContractOutcomeStore(); + var row = CreatePrototypeOutcome(); + Assert.True(store.TryAppend(row)); + // Act + var duplicateId = store.TryAppend(row with { IdempotencyKey = "other-key" }); + // Assert + Assert.False(duplicateId); + } + + [Fact] + public void TryAppend_ShouldDenyDuplicateIdempotencyKey() + { + // Arrange + var store = new InMemoryContractOutcomeStore(); + var row = CreatePrototypeOutcome(); + Assert.True(store.TryAppend(row)); + // Act + var duplicateKey = store.TryAppend(row with { Id = "outcome-row-002" }); + // Assert + Assert.False(duplicateKey); + } + + [Fact] + public void TryAppend_ShouldFailClosed_WhenRequiredFieldsEmpty() + { + // Arrange + var store = new InMemoryContractOutcomeStore(); + var invalidRow = CreatePrototypeOutcome() with { PlayerId = " " }; + // Act + var appended = store.TryAppend(invalidRow); + // Assert + Assert.False(appended); + Assert.Empty(store.GetOutcomesForInstance(InstanceId)); + } + + private static ContractOutcomeRow CreatePrototypeOutcome() + { + var idempotencyKey = ContractOutcomeIds.MakeIdempotencyKey(PlayerId, InstanceId); + return new ContractOutcomeRow( + OutcomeRowId, + InstanceId, + PlayerId, + idempotencyKey, + [new RewardItemGrantApplied("scrap_metal_bulk", 5)], + [new RewardSkillXpGrantApplied("salvage", 15)], + [], + RecordedAt); + } +} diff --git a/server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs b/server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs index cd8fb67..e81dc80 100644 --- a/server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs +++ b/server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs @@ -96,6 +96,8 @@ public sealed class InMemoryWebApplicationFactory : WebApplicationFactory(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); diff --git a/server/NeonSprawl.Server/Game/Contracts/ContractInstanceIds.cs b/server/NeonSprawl.Server/Game/Contracts/ContractInstanceIds.cs new file mode 100644 index 0000000..3317557 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Contracts/ContractInstanceIds.cs @@ -0,0 +1,43 @@ +namespace NeonSprawl.Server.Game.Contracts; + +/// Id normalization for contract instance stores (NEO-146). +public static class ContractInstanceIds +{ + /// Trim + lowercase; empty when input is null/whitespace. + public static string NormalizePlayerId(string? playerId) + { + var trimmed = playerId?.Trim(); + if (string.IsNullOrEmpty(trimmed)) + { + return string.Empty; + } + + return trimmed.ToLowerInvariant(); + } + + /// Trim + lowercase; empty when input is null/whitespace. + public static string NormalizeContractInstanceId(string? contractInstanceId) => NormalizePlayerId(contractInstanceId); + + /// Trim + lowercase; empty when input is null/whitespace. + public static string NormalizeTemplateId(string? templateId) => NormalizePlayerId(templateId); + + /// Trim; empty when input is null/whitespace (seed bucket casing preserved). + public static string NormalizeSeedBucket(string? seedBucket) + { + var trimmed = seedBucket?.Trim(); + return string.IsNullOrEmpty(trimmed) ? string.Empty : trimmed; + } + + /// Composite store key for + . + public static string MakeInstanceKey(string? playerId, string? contractInstanceId) + { + var p = NormalizePlayerId(playerId); + var i = NormalizeContractInstanceId(contractInstanceId); + if (p.Length == 0 || i.Length == 0) + { + return string.Empty; + } + + return $"{p}\0{i}"; + } +} diff --git a/server/NeonSprawl.Server/Game/Contracts/ContractInstanceReasonCodes.cs b/server/NeonSprawl.Server/Game/Contracts/ContractInstanceReasonCodes.cs new file mode 100644 index 0000000..899c686 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Contracts/ContractInstanceReasonCodes.cs @@ -0,0 +1,15 @@ +namespace NeonSprawl.Server.Game.Contracts; + +/// Stable deny reason codes for contract instance operations (NEO-146; consumed by E7M4-04+ orchestrators). +public static class ContractInstanceReasonCodes +{ + public const string PlayerNotWritable = "player_not_writable"; + + public const string ActiveContractExists = "active_contract_exists"; + + public const string InstanceNotFound = "instance_not_found"; + + public const string InstanceNotActive = "instance_not_active"; + + public const string InvalidIds = "invalid_ids"; +} diff --git a/server/NeonSprawl.Server/Game/Contracts/ContractInstanceServiceCollectionExtensions.cs b/server/NeonSprawl.Server/Game/Contracts/ContractInstanceServiceCollectionExtensions.cs new file mode 100644 index 0000000..a835f34 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Contracts/ContractInstanceServiceCollectionExtensions.cs @@ -0,0 +1,25 @@ +using NeonSprawl.Server.Game.PositionState; + +namespace NeonSprawl.Server.Game.Contracts; + +/// Registers contract instance + outcome persistence (NEO-146). +public static class ContractInstanceServiceCollectionExtensions +{ + /// PostgreSQL when ConnectionStrings:NeonSprawl is set; otherwise in-memory fallback. + public static IServiceCollection AddContractInstanceStores(this IServiceCollection services, IConfiguration configuration) + { + var cs = configuration.GetConnectionString(PositionStateServiceCollectionExtensions.NeonSprawlConnectionStringName); + if (!string.IsNullOrWhiteSpace(cs)) + { + services.AddSingleton(); + services.AddSingleton(); + } + else + { + services.AddSingleton(); + services.AddSingleton(); + } + + return services; + } +} diff --git a/server/NeonSprawl.Server/Game/Contracts/ContractInstanceState.cs b/server/NeonSprawl.Server/Game/Contracts/ContractInstanceState.cs new file mode 100644 index 0000000..848b177 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Contracts/ContractInstanceState.cs @@ -0,0 +1,26 @@ +namespace NeonSprawl.Server.Game.Contracts; + +/// Immutable per-player contract instance snapshot (NEO-146). +public sealed class ContractInstanceState( + string contractInstanceId, + string templateId, + string playerId, + ContractInstanceStatus status, + string seedBucket, + DateTimeOffset issuedAt, + DateTimeOffset? completedAt) +{ + public string ContractInstanceId { get; } = contractInstanceId; + + public string TemplateId { get; } = templateId; + + public string PlayerId { get; } = playerId; + + public ContractInstanceStatus Status { get; } = status; + + public string SeedBucket { get; } = seedBucket; + + public DateTimeOffset IssuedAt { get; } = issuedAt; + + public DateTimeOffset? CompletedAt { get; } = completedAt; +} diff --git a/server/NeonSprawl.Server/Game/Contracts/ContractInstanceStatus.cs b/server/NeonSprawl.Server/Game/Contracts/ContractInstanceStatus.cs new file mode 100644 index 0000000..f3e42e6 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Contracts/ContractInstanceStatus.cs @@ -0,0 +1,9 @@ +namespace NeonSprawl.Server.Game.Contracts; + +/// Contract instance lifecycle status (NEO-146). +public enum ContractInstanceStatus +{ + Active, + Completed, + Expired, +} diff --git a/server/NeonSprawl.Server/Game/Contracts/ContractOutcomeIds.cs b/server/NeonSprawl.Server/Game/Contracts/ContractOutcomeIds.cs new file mode 100644 index 0000000..7f958c7 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Contracts/ContractOutcomeIds.cs @@ -0,0 +1,18 @@ +namespace NeonSprawl.Server.Game.Contracts; + +/// Id normalization and idempotency keys for contract outcome audit rows (NEO-146). +public static class ContractOutcomeIds +{ + /// Stable idempotency key for contract completion delivery. + public static string MakeIdempotencyKey(string? playerId, string? contractInstanceId) + { + var p = ContractInstanceIds.NormalizePlayerId(playerId); + var i = ContractInstanceIds.NormalizeContractInstanceId(contractInstanceId); + if (p.Length == 0 || i.Length == 0) + { + return string.Empty; + } + + return $"{p}:contract_complete:{i}"; + } +} diff --git a/server/NeonSprawl.Server/Game/Contracts/ContractOutcomeRow.cs b/server/NeonSprawl.Server/Game/Contracts/ContractOutcomeRow.cs new file mode 100644 index 0000000..155cf22 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Contracts/ContractOutcomeRow.cs @@ -0,0 +1,14 @@ +using NeonSprawl.Server.Game.Rewards; + +namespace NeonSprawl.Server.Game.Contracts; + +/// Append-only contract completion outcome audit row (NEO-146). +public sealed record ContractOutcomeRow( + string Id, + string ContractInstanceId, + string PlayerId, + string IdempotencyKey, + IReadOnlyList GrantedItems, + IReadOnlyList GrantedSkillXp, + IReadOnlyList GrantedReputation, + DateTimeOffset RecordedAt); diff --git a/server/NeonSprawl.Server/Game/Contracts/IContractInstanceStore.cs b/server/NeonSprawl.Server/Game/Contracts/IContractInstanceStore.cs new file mode 100644 index 0000000..6bb48b6 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Contracts/IContractInstanceStore.cs @@ -0,0 +1,39 @@ +namespace NeonSprawl.Server.Game.Contracts; + +/// +/// Persisted per-player contract instances (NEO-146). +/// Contract generator/completion orchestrators (NEO-147, NEO-149) consume this interface — not HTTP directly. +/// +public interface IContractInstanceStore +{ + /// True when mutations for are allowed (dev bucket or Postgres player_position row). + bool CanWritePlayer(string playerId); + + /// Missing row ⇒ false. + bool TryGet(string playerId, string contractInstanceId, out ContractInstanceState snapshot); + + /// At most one active row per player; missing active ⇒ false. + bool TryGetActiveForPlayer(string playerId, out ContractInstanceState snapshot); + + /// + /// Creates an active instance. Returns false when player not writable, ids invalid, duplicate instance id, + /// or player already has an active contract. + /// + bool TryCreateActive( + string playerId, + string contractInstanceId, + string templateId, + string seedBucket, + DateTimeOffset issuedAt, + out ContractInstanceState snapshot); + + /// First completion returns true; replays return false without changing . + bool TryMarkComplete( + string playerId, + string contractInstanceId, + DateTimeOffset completedAt, + out ContractInstanceState snapshot); + + /// Dev fixture only — removes one instance row; idempotent when absent. + bool TryClearInstance(string playerId, string contractInstanceId); +} diff --git a/server/NeonSprawl.Server/Game/Contracts/IContractOutcomeStore.cs b/server/NeonSprawl.Server/Game/Contracts/IContractOutcomeStore.cs new file mode 100644 index 0000000..afbeb0a --- /dev/null +++ b/server/NeonSprawl.Server/Game/Contracts/IContractOutcomeStore.cs @@ -0,0 +1,16 @@ +namespace NeonSprawl.Server.Game.Contracts; + +/// Append-only contract completion outcome audit log (NEO-146). +public interface IContractOutcomeStore +{ + /// First append with a given row Id or IdempotencyKey returns true; duplicates return false. + bool TryAppend(ContractOutcomeRow row); + + bool TryGetByIdempotencyKey(string idempotencyKey, out ContractOutcomeRow row); + + /// Audit rows for one instance ordered by then Id ascending. + IReadOnlyList GetOutcomesForInstance(string contractInstanceId, int? limit = null); + + /// Dev fixture only — removes outcome rows for one instance; idempotent when none match. + bool TryClearForInstance(string contractInstanceId); +} diff --git a/server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs b/server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs new file mode 100644 index 0000000..8b90fb2 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs @@ -0,0 +1,250 @@ +using System.Collections.Concurrent; +using Microsoft.Extensions.Options; +using NeonSprawl.Server.Game.PositionState; + +namespace NeonSprawl.Server.Game.Contracts; + +/// Thread-safe in-memory contract instances; seeds the configured dev player (NEO-146). +public sealed class InMemoryContractInstanceStore(IOptions options) : IContractInstanceStore +{ + private sealed class InstanceRow( + string contractInstanceId, + string templateId, + string playerId, + ContractInstanceStatus status, + string seedBucket, + DateTimeOffset issuedAt, + DateTimeOffset? completedAt) + { + public string ContractInstanceId { get; } = contractInstanceId; + + public string TemplateId { get; } = templateId; + + public string PlayerId { get; } = playerId; + + public ContractInstanceStatus Status { get; private set; } = status; + + public string SeedBucket { get; } = seedBucket; + + public DateTimeOffset IssuedAt { get; } = issuedAt; + + public DateTimeOffset? CompletedAt { get; private set; } = completedAt; + + public ContractInstanceState ToSnapshot() => + new(ContractInstanceId, TemplateId, PlayerId, Status, SeedBucket, IssuedAt, CompletedAt); + + public void MarkCompleted(DateTimeOffset completedAt) + { + Status = ContractInstanceStatus.Completed; + CompletedAt = completedAt; + } + } + + private readonly HashSet knownPlayers = CreateKnownPlayers(options.Value); + + private readonly ConcurrentDictionary byInstanceId = new(StringComparer.Ordinal); + + private readonly ConcurrentDictionary activeInstanceByPlayer = new(StringComparer.Ordinal); + + private readonly ConcurrentDictionary instanceLocks = new(StringComparer.Ordinal); + + private static HashSet CreateKnownPlayers(GamePositionOptions o) + { + var id = ContractInstanceIds.NormalizePlayerId(o.DevPlayerId); + if (id.Length == 0) + { + throw new InvalidOperationException("Game:DevPlayerId must be non-empty."); + } + + return new HashSet(StringComparer.OrdinalIgnoreCase) { id }; + } + + /// + public bool CanWritePlayer(string playerId) + { + var player = ContractInstanceIds.NormalizePlayerId(playerId); + return player.Length > 0 && knownPlayers.Contains(player); + } + + /// + public bool TryGet(string playerId, string contractInstanceId, out ContractInstanceState snapshot) + { + snapshot = null!; + var player = ContractInstanceIds.NormalizePlayerId(playerId); + var instanceId = ContractInstanceIds.NormalizeContractInstanceId(contractInstanceId); + if (player.Length == 0 || instanceId.Length == 0 || !knownPlayers.Contains(player)) + { + return false; + } + + lock (instanceLocks.GetOrAdd(instanceId, _ => new object())) + { + if (!byInstanceId.TryGetValue(instanceId, out var row) || + !string.Equals(row.PlayerId, player, StringComparison.Ordinal)) + { + return false; + } + + snapshot = row.ToSnapshot(); + return true; + } + } + + /// + public bool TryGetActiveForPlayer(string playerId, out ContractInstanceState snapshot) + { + snapshot = null!; + var player = ContractInstanceIds.NormalizePlayerId(playerId); + if (player.Length == 0 || !knownPlayers.Contains(player)) + { + return false; + } + + if (!activeInstanceByPlayer.TryGetValue(player, out var instanceId)) + { + return false; + } + + lock (instanceLocks.GetOrAdd(instanceId, _ => new object())) + { + if (!byInstanceId.TryGetValue(instanceId, out var row) || + row.Status != ContractInstanceStatus.Active || + !string.Equals(row.PlayerId, player, StringComparison.Ordinal)) + { + activeInstanceByPlayer.TryRemove(player, out _); + return false; + } + + snapshot = row.ToSnapshot(); + return true; + } + } + + /// + public bool TryCreateActive( + string playerId, + string contractInstanceId, + string templateId, + string seedBucket, + DateTimeOffset issuedAt, + out ContractInstanceState snapshot) + { + snapshot = null!; + var player = ContractInstanceIds.NormalizePlayerId(playerId); + var instanceId = ContractInstanceIds.NormalizeContractInstanceId(contractInstanceId); + var normalizedTemplateId = ContractInstanceIds.NormalizeTemplateId(templateId); + var normalizedSeedBucket = ContractInstanceIds.NormalizeSeedBucket(seedBucket); + if (player.Length == 0 || + instanceId.Length == 0 || + normalizedTemplateId.Length == 0 || + normalizedSeedBucket.Length == 0 || + !knownPlayers.Contains(player)) + { + return false; + } + + lock (instanceLocks.GetOrAdd(instanceId, _ => new object())) + { + if (activeInstanceByPlayer.TryGetValue(player, out var existingActiveId)) + { + if (byInstanceId.TryGetValue(existingActiveId, out var existingActive) && + existingActive.Status == ContractInstanceStatus.Active) + { + snapshot = existingActive.ToSnapshot(); + return false; + } + + activeInstanceByPlayer.TryRemove(player, out _); + } + + if (byInstanceId.TryGetValue(instanceId, out var existing)) + { + snapshot = existing.ToSnapshot(); + return false; + } + + var row = new InstanceRow( + instanceId, + normalizedTemplateId, + player, + ContractInstanceStatus.Active, + normalizedSeedBucket, + issuedAt, + null); + byInstanceId[instanceId] = row; + activeInstanceByPlayer[player] = instanceId; + snapshot = row.ToSnapshot(); + return true; + } + } + + /// + public bool TryMarkComplete( + string playerId, + string contractInstanceId, + DateTimeOffset completedAt, + out ContractInstanceState snapshot) + { + snapshot = null!; + var player = ContractInstanceIds.NormalizePlayerId(playerId); + var instanceId = ContractInstanceIds.NormalizeContractInstanceId(contractInstanceId); + if (player.Length == 0 || instanceId.Length == 0 || !knownPlayers.Contains(player)) + { + return false; + } + + lock (instanceLocks.GetOrAdd(instanceId, _ => new object())) + { + if (!byInstanceId.TryGetValue(instanceId, out var row) || + !string.Equals(row.PlayerId, player, StringComparison.Ordinal)) + { + return false; + } + + if (row.Status == ContractInstanceStatus.Completed) + { + snapshot = row.ToSnapshot(); + return false; + } + + if (row.Status != ContractInstanceStatus.Active) + { + snapshot = row.ToSnapshot(); + return false; + } + + row.MarkCompleted(completedAt); + activeInstanceByPlayer.TryRemove(player, out _); + snapshot = row.ToSnapshot(); + return true; + } + } + + /// + public bool TryClearInstance(string playerId, string contractInstanceId) + { + var player = ContractInstanceIds.NormalizePlayerId(playerId); + var instanceId = ContractInstanceIds.NormalizeContractInstanceId(contractInstanceId); + if (player.Length == 0 || instanceId.Length == 0 || !knownPlayers.Contains(player)) + { + return false; + } + + lock (instanceLocks.GetOrAdd(instanceId, _ => new object())) + { + if (!byInstanceId.TryGetValue(instanceId, out var row) || + !string.Equals(row.PlayerId, player, StringComparison.Ordinal)) + { + return false; + } + + _ = byInstanceId.TryRemove(instanceId, out _); + if (row.Status == ContractInstanceStatus.Active) + { + activeInstanceByPlayer.TryRemove(player, out _); + } + + return true; + } + } +} diff --git a/server/NeonSprawl.Server/Game/Contracts/InMemoryContractOutcomeStore.cs b/server/NeonSprawl.Server/Game/Contracts/InMemoryContractOutcomeStore.cs new file mode 100644 index 0000000..842e2f1 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Contracts/InMemoryContractOutcomeStore.cs @@ -0,0 +1,133 @@ +using System.Collections.Concurrent; +using NeonSprawl.Server.Game.Rewards; + +namespace NeonSprawl.Server.Game.Contracts; + +/// Thread-safe in-memory append-only contract outcome audit log (NEO-146). +public sealed class InMemoryContractOutcomeStore : IContractOutcomeStore +{ + private readonly ConcurrentDictionary rowsById = new(StringComparer.Ordinal); + + private readonly ConcurrentDictionary rowIdByIdempotencyKey = new(StringComparer.Ordinal); + + private readonly ConcurrentDictionary idLocks = new(StringComparer.Ordinal); + + /// + public bool TryAppend(ContractOutcomeRow row) + { + if (!TryNormalizeRow(row, out var normalized)) + { + return false; + } + + lock (idLocks.GetOrAdd(normalized.Id, _ => new object())) + { + if (rowsById.ContainsKey(normalized.Id) || + rowIdByIdempotencyKey.ContainsKey(normalized.IdempotencyKey)) + { + return false; + } + + rowsById[normalized.Id] = normalized; + rowIdByIdempotencyKey[normalized.IdempotencyKey] = normalized.Id; + return true; + } + } + + /// + public bool TryGetByIdempotencyKey(string idempotencyKey, out ContractOutcomeRow row) + { + row = null!; + var key = idempotencyKey?.Trim(); + if (string.IsNullOrEmpty(key)) + { + return false; + } + + if (!rowIdByIdempotencyKey.TryGetValue(key, out var rowId) || + !rowsById.TryGetValue(rowId, out var stored)) + { + return false; + } + + row = stored; + return true; + } + + /// + public IReadOnlyList GetOutcomesForInstance(string contractInstanceId, int? limit = null) + { + var instanceId = ContractInstanceIds.NormalizeContractInstanceId(contractInstanceId); + if (instanceId.Length == 0) + { + return []; + } + + IEnumerable query = rowsById.Values + .Where(r => string.Equals(r.ContractInstanceId, instanceId, StringComparison.Ordinal)) + .OrderBy(static r => r.RecordedAt) + .ThenBy(static r => r.Id, StringComparer.Ordinal); + + if (limit is > 0) + { + query = query.Take(limit.Value); + } + + return query.ToArray(); + } + + /// + public bool TryClearForInstance(string contractInstanceId) + { + var instanceId = ContractInstanceIds.NormalizeContractInstanceId(contractInstanceId); + if (instanceId.Length == 0) + { + return false; + } + + var idsToRemove = rowsById.Values + .Where(row => string.Equals(row.ContractInstanceId, instanceId, StringComparison.Ordinal)) + .Select(static row => row.Id) + .ToArray(); + + foreach (var id in idsToRemove) + { + if (rowsById.TryRemove(id, out var removed)) + { + rowIdByIdempotencyKey.TryRemove(removed.IdempotencyKey, out _); + } + + idLocks.TryRemove(id, out _); + } + + return true; + } + + private static bool TryNormalizeRow(ContractOutcomeRow row, out ContractOutcomeRow normalized) + { + normalized = null!; + var id = row.Id.Trim(); + var playerId = ContractInstanceIds.NormalizePlayerId(row.PlayerId); + var instanceId = ContractInstanceIds.NormalizeContractInstanceId(row.ContractInstanceId); + var idempotencyKey = row.IdempotencyKey.Trim(); + if (id.Length == 0 || + playerId.Length == 0 || + instanceId.Length == 0 || + idempotencyKey.Length == 0) + { + return false; + } + + normalized = row with + { + Id = id, + PlayerId = playerId, + ContractInstanceId = instanceId, + IdempotencyKey = idempotencyKey, + GrantedItems = row.GrantedItems.ToArray(), + GrantedSkillXp = row.GrantedSkillXp.ToArray(), + GrantedReputation = row.GrantedReputation.ToArray(), + }; + return true; + } +} diff --git a/server/NeonSprawl.Server/Game/Contracts/PostgresContractInstanceBootstrap.cs b/server/NeonSprawl.Server/Game/Contracts/PostgresContractInstanceBootstrap.cs new file mode 100644 index 0000000..c2e7de6 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Contracts/PostgresContractInstanceBootstrap.cs @@ -0,0 +1,39 @@ +namespace NeonSprawl.Server.Game.Contracts; + +/// Applies NEO-146 contract instance table DDL once per process. +public static class PostgresContractInstanceBootstrap +{ + private static readonly string DdlRelativePath = Path.Combine("db", "migrations", "V011__contract_instance.sql"); + + private static readonly object SchemaGate = new(); + + private static int _schemaReady; + + public static void EnsureSchema(Npgsql.NpgsqlDataSource dataSource) + { + if (Volatile.Read(ref _schemaReady) != 0) + { + return; + } + + lock (SchemaGate) + { + if (Volatile.Read(ref _schemaReady) != 0) + { + return; + } + + var ddlPath = Path.Combine(AppContext.BaseDirectory, DdlRelativePath); + if (!File.Exists(ddlPath)) + { + throw new FileNotFoundException($"NEO-146 contract instance DDL not found at '{ddlPath}'.", ddlPath); + } + + var ddl = File.ReadAllText(ddlPath); + using var conn = dataSource.OpenConnection(); + using var cmd = new Npgsql.NpgsqlCommand(ddl, conn); + cmd.ExecuteNonQuery(); + Volatile.Write(ref _schemaReady, 1); + } + } +} diff --git a/server/NeonSprawl.Server/Game/Contracts/PostgresContractInstanceStore.cs b/server/NeonSprawl.Server/Game/Contracts/PostgresContractInstanceStore.cs new file mode 100644 index 0000000..99945fc --- /dev/null +++ b/server/NeonSprawl.Server/Game/Contracts/PostgresContractInstanceStore.cs @@ -0,0 +1,375 @@ +namespace NeonSprawl.Server.Game.Contracts; + +/// PostgreSQL-backed contract instances keyed by normalized contract instance id (NEO-146). +public sealed class PostgresContractInstanceStore(Npgsql.NpgsqlDataSource dataSource) : IContractInstanceStore +{ + /// + public bool CanWritePlayer(string playerId) + { + var player = ContractInstanceIds.NormalizePlayerId(playerId); + if (player.Length == 0) + { + return false; + } + + PostgresContractInstanceBootstrap.EnsureSchema(dataSource); + using var conn = dataSource.OpenConnection(); + return PlayerExists(conn, player); + } + + /// + public bool TryGet(string playerId, string contractInstanceId, out ContractInstanceState snapshot) + { + snapshot = null!; + var player = ContractInstanceIds.NormalizePlayerId(playerId); + var instanceId = ContractInstanceIds.NormalizeContractInstanceId(contractInstanceId); + if (player.Length == 0 || instanceId.Length == 0) + { + return false; + } + + PostgresContractInstanceBootstrap.EnsureSchema(dataSource); + using var conn = dataSource.OpenConnection(); + if (!PlayerExists(conn, player)) + { + return false; + } + + using var cmd = new Npgsql.NpgsqlCommand( + """ + SELECT contract_instance_id, template_id, player_id, status, seed_bucket, issued_at, completed_at + FROM contract_instance + WHERE contract_instance_id = @id AND player_id = @pid + LIMIT 1; + """, + conn); + cmd.Parameters.AddWithValue("id", instanceId); + cmd.Parameters.AddWithValue("pid", player); + using var reader = cmd.ExecuteReader(); + if (!reader.Read()) + { + return false; + } + + snapshot = ReadSnapshot(reader); + return true; + } + + /// + public bool TryGetActiveForPlayer(string playerId, out ContractInstanceState snapshot) + { + snapshot = null!; + var player = ContractInstanceIds.NormalizePlayerId(playerId); + if (player.Length == 0) + { + return false; + } + + PostgresContractInstanceBootstrap.EnsureSchema(dataSource); + using var conn = dataSource.OpenConnection(); + if (!PlayerExists(conn, player)) + { + return false; + } + + using var cmd = new Npgsql.NpgsqlCommand( + """ + SELECT contract_instance_id, template_id, player_id, status, seed_bucket, issued_at, completed_at + FROM contract_instance + WHERE player_id = @pid AND status = 'active' + LIMIT 1; + """, + conn); + cmd.Parameters.AddWithValue("pid", player); + using var reader = cmd.ExecuteReader(); + if (!reader.Read()) + { + return false; + } + + snapshot = ReadSnapshot(reader); + return snapshot.Status == ContractInstanceStatus.Active; + } + + /// + public bool TryCreateActive( + string playerId, + string contractInstanceId, + string templateId, + string seedBucket, + DateTimeOffset issuedAt, + out ContractInstanceState snapshot) + { + snapshot = null!; + var player = ContractInstanceIds.NormalizePlayerId(playerId); + var instanceId = ContractInstanceIds.NormalizeContractInstanceId(contractInstanceId); + var normalizedTemplateId = ContractInstanceIds.NormalizeTemplateId(templateId); + var normalizedSeedBucket = ContractInstanceIds.NormalizeSeedBucket(seedBucket); + if (player.Length == 0 || + instanceId.Length == 0 || + normalizedTemplateId.Length == 0 || + normalizedSeedBucket.Length == 0) + { + return false; + } + + PostgresContractInstanceBootstrap.EnsureSchema(dataSource); + using var conn = dataSource.OpenConnection(); + using var tx = conn.BeginTransaction(); + if (!PlayerExists(conn, player, tx)) + { + tx.Rollback(); + return false; + } + + ContractInstanceState? activeSnapshot = null; + using (var activeSel = new Npgsql.NpgsqlCommand( + """ + SELECT contract_instance_id, template_id, player_id, status, seed_bucket, issued_at, completed_at + FROM contract_instance + WHERE player_id = @pid AND status = 'active' + LIMIT 1 + FOR UPDATE; + """, + conn, + tx)) + { + activeSel.Parameters.AddWithValue("pid", player); + using var reader = activeSel.ExecuteReader(); + if (reader.Read()) + { + activeSnapshot = ReadSnapshot(reader); + } + } + + if (activeSnapshot is not null) + { + snapshot = activeSnapshot; + tx.Rollback(); + return false; + } + + using var insert = new Npgsql.NpgsqlCommand( + """ + INSERT INTO contract_instance ( + contract_instance_id, player_id, template_id, status, seed_bucket, issued_at, updated_at) + VALUES (@id, @pid, @tid, 'active', @seed, @issued, now()) + ON CONFLICT (contract_instance_id) DO NOTHING; + """, + conn, + tx); + insert.Parameters.AddWithValue("id", instanceId); + insert.Parameters.AddWithValue("pid", player); + insert.Parameters.AddWithValue("tid", normalizedTemplateId); + insert.Parameters.AddWithValue("seed", normalizedSeedBucket); + insert.Parameters.AddWithValue("issued", issuedAt); + try + { + if (insert.ExecuteNonQuery() > 0) + { + tx.Commit(); + snapshot = new ContractInstanceState( + instanceId, + normalizedTemplateId, + player, + ContractInstanceStatus.Active, + normalizedSeedBucket, + issuedAt, + null); + return true; + } + } + catch (Npgsql.PostgresException ex) when (ex.SqlState == Npgsql.PostgresErrorCodes.UniqueViolation) + { + tx.Rollback(); + return TryReadDenySnapshot(player, instanceId, out snapshot); + } + + using (var existingSel = new Npgsql.NpgsqlCommand( + """ + SELECT contract_instance_id, template_id, player_id, status, seed_bucket, issued_at, completed_at + FROM contract_instance + WHERE contract_instance_id = @id + LIMIT 1; + """, + conn, + tx)) + { + existingSel.Parameters.AddWithValue("id", instanceId); + using var reader = existingSel.ExecuteReader(); + if (reader.Read()) + { + snapshot = ReadSnapshot(reader); + } + } + + tx.Rollback(); + return false; + } + + private bool TryReadDenySnapshot(string player, string instanceId, out ContractInstanceState snapshot) + { + if (TryGet(player, instanceId, out snapshot)) + { + return false; + } + + if (TryGetActiveForPlayer(player, out snapshot)) + { + return false; + } + + snapshot = null!; + return false; + } + + /// + public bool TryMarkComplete( + string playerId, + string contractInstanceId, + DateTimeOffset completedAt, + out ContractInstanceState snapshot) + { + snapshot = null!; + var player = ContractInstanceIds.NormalizePlayerId(playerId); + var instanceId = ContractInstanceIds.NormalizeContractInstanceId(contractInstanceId); + if (player.Length == 0 || instanceId.Length == 0) + { + return false; + } + + PostgresContractInstanceBootstrap.EnsureSchema(dataSource); + using var conn = dataSource.OpenConnection(); + using var tx = conn.BeginTransaction(); + if (!PlayerExists(conn, player, tx)) + { + tx.Rollback(); + return false; + } + + ContractInstanceState current; + using (var sel = new Npgsql.NpgsqlCommand( + """ + SELECT contract_instance_id, template_id, player_id, status, seed_bucket, issued_at, completed_at + FROM contract_instance + WHERE contract_instance_id = @id AND player_id = @pid + LIMIT 1 + FOR UPDATE; + """, + conn, + tx)) + { + sel.Parameters.AddWithValue("id", instanceId); + sel.Parameters.AddWithValue("pid", player); + using var reader = sel.ExecuteReader(); + if (!reader.Read()) + { + tx.Rollback(); + return false; + } + + current = ReadSnapshot(reader); + } + + if (current.Status == ContractInstanceStatus.Completed) + { + snapshot = current; + tx.Rollback(); + return false; + } + + if (current.Status != ContractInstanceStatus.Active) + { + snapshot = current; + tx.Rollback(); + return false; + } + + using (var update = new Npgsql.NpgsqlCommand( + """ + UPDATE contract_instance + SET status = 'completed', + completed_at = @completed, + updated_at = now() + WHERE contract_instance_id = @id AND player_id = @pid; + """, + conn, + tx)) + { + update.Parameters.AddWithValue("id", instanceId); + update.Parameters.AddWithValue("pid", player); + update.Parameters.AddWithValue("completed", completedAt); + update.ExecuteNonQuery(); + } + + tx.Commit(); + snapshot = new ContractInstanceState( + current.ContractInstanceId, + current.TemplateId, + current.PlayerId, + ContractInstanceStatus.Completed, + current.SeedBucket, + current.IssuedAt, + completedAt); + return true; + } + + /// + public bool TryClearInstance(string playerId, string contractInstanceId) + { + var player = ContractInstanceIds.NormalizePlayerId(playerId); + var instanceId = ContractInstanceIds.NormalizeContractInstanceId(contractInstanceId); + if (player.Length == 0 || instanceId.Length == 0 || !CanWritePlayer(player)) + { + return false; + } + + PostgresContractInstanceBootstrap.EnsureSchema(dataSource); + using var conn = dataSource.OpenConnection(); + using var cmd = new Npgsql.NpgsqlCommand( + """ + DELETE FROM contract_instance + WHERE contract_instance_id = @id AND player_id = @pid; + """, + conn); + cmd.Parameters.AddWithValue("id", instanceId); + cmd.Parameters.AddWithValue("pid", player); + cmd.ExecuteNonQuery(); + return true; + } + + private static ContractInstanceState ReadSnapshot(Npgsql.NpgsqlDataReader reader) => + new( + reader.GetString(0), + reader.GetString(1), + reader.GetString(2), + ParseStatus(reader.GetString(3)), + reader.GetString(4), + reader.GetFieldValue(5), + reader.IsDBNull(6) ? null : reader.GetFieldValue(6)); + + private static ContractInstanceStatus ParseStatus(string raw) => + raw switch + { + "active" => ContractInstanceStatus.Active, + "completed" => ContractInstanceStatus.Completed, + "expired" => ContractInstanceStatus.Expired, + _ => throw new InvalidOperationException($"Unknown contract instance status '{raw}'."), + }; + + private static bool PlayerExists(Npgsql.NpgsqlConnection conn, string playerIdNormalized) => + PlayerExists(conn, playerIdNormalized, null); + + private static bool PlayerExists( + Npgsql.NpgsqlConnection conn, + string playerIdNormalized, + Npgsql.NpgsqlTransaction? tx) + { + using var cmd = new Npgsql.NpgsqlCommand( + "SELECT 1 FROM player_position WHERE player_id = @pid LIMIT 1;", + conn, + tx); + cmd.Parameters.AddWithValue("pid", playerIdNormalized); + return cmd.ExecuteScalar() is not null; + } +} diff --git a/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeBootstrap.cs b/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeBootstrap.cs new file mode 100644 index 0000000..a85e846 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeBootstrap.cs @@ -0,0 +1,39 @@ +namespace NeonSprawl.Server.Game.Contracts; + +/// Applies NEO-146 contract outcome table DDL once per process. +public static class PostgresContractOutcomeBootstrap +{ + private static readonly string DdlRelativePath = Path.Combine("db", "migrations", "V012__contract_outcome.sql"); + + private static readonly object SchemaGate = new(); + + private static int _schemaReady; + + public static void EnsureSchema(Npgsql.NpgsqlDataSource dataSource) + { + if (Volatile.Read(ref _schemaReady) != 0) + { + return; + } + + lock (SchemaGate) + { + if (Volatile.Read(ref _schemaReady) != 0) + { + return; + } + + var ddlPath = Path.Combine(AppContext.BaseDirectory, DdlRelativePath); + if (!File.Exists(ddlPath)) + { + throw new FileNotFoundException($"NEO-146 contract outcome DDL not found at '{ddlPath}'.", ddlPath); + } + + var ddl = File.ReadAllText(ddlPath); + using var conn = dataSource.OpenConnection(); + using var cmd = new Npgsql.NpgsqlCommand(ddl, conn); + cmd.ExecuteNonQuery(); + Volatile.Write(ref _schemaReady, 1); + } + } +} diff --git a/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs b/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs new file mode 100644 index 0000000..222c05c --- /dev/null +++ b/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs @@ -0,0 +1,203 @@ +using System.Text.Json; +using NeonSprawl.Server.Game.Rewards; + +namespace NeonSprawl.Server.Game.Contracts; + +/// PostgreSQL-backed append-only contract outcome audit log (NEO-146). +public sealed class PostgresContractOutcomeStore(Npgsql.NpgsqlDataSource dataSource) : IContractOutcomeStore +{ + private static readonly JsonSerializerOptions JsonOptions = new() + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + }; + + /// + public bool TryAppend(ContractOutcomeRow row) + { + if (!TryNormalizeRow(row, out var normalized)) + { + return false; + } + + PostgresContractOutcomeBootstrap.EnsureSchema(dataSource); + using var conn = dataSource.OpenConnection(); + using var cmd = new Npgsql.NpgsqlCommand( + """ + INSERT INTO contract_outcome ( + id, contract_instance_id, player_id, idempotency_key, + granted_items, granted_skill_xp, granted_reputation, recorded_at) + SELECT @id, @instance_id, @pid, @key, @items::jsonb, @skill_xp::jsonb, @rep::jsonb, @recorded + WHERE NOT EXISTS ( + SELECT 1 FROM contract_outcome + WHERE id = @id OR idempotency_key = @key); + """, + conn); + cmd.Parameters.AddWithValue("id", normalized.Id); + cmd.Parameters.AddWithValue("instance_id", normalized.ContractInstanceId); + cmd.Parameters.AddWithValue("pid", normalized.PlayerId); + cmd.Parameters.AddWithValue("key", normalized.IdempotencyKey); + cmd.Parameters.AddWithValue("items", SerializeGrants(normalized.GrantedItems)); + cmd.Parameters.AddWithValue("skill_xp", SerializeGrants(normalized.GrantedSkillXp)); + cmd.Parameters.AddWithValue("rep", SerializeGrants(normalized.GrantedReputation)); + cmd.Parameters.AddWithValue("recorded", normalized.RecordedAt); + return cmd.ExecuteNonQuery() == 1; + } + + /// + public bool TryGetByIdempotencyKey(string idempotencyKey, out ContractOutcomeRow row) + { + row = null!; + var key = idempotencyKey?.Trim(); + if (string.IsNullOrEmpty(key)) + { + return false; + } + + PostgresContractOutcomeBootstrap.EnsureSchema(dataSource); + using var conn = dataSource.OpenConnection(); + using var cmd = new Npgsql.NpgsqlCommand( + """ + SELECT id, contract_instance_id, player_id, idempotency_key, + granted_items, granted_skill_xp, granted_reputation, recorded_at + FROM contract_outcome + WHERE idempotency_key = @key + LIMIT 1; + """, + conn); + cmd.Parameters.AddWithValue("key", key); + using var reader = cmd.ExecuteReader(); + if (!reader.Read()) + { + return false; + } + + row = ReadRow(reader); + return true; + } + + /// + public IReadOnlyList GetOutcomesForInstance(string contractInstanceId, int? limit = null) + { + var instanceId = ContractInstanceIds.NormalizeContractInstanceId(contractInstanceId); + if (instanceId.Length == 0) + { + return []; + } + + PostgresContractOutcomeBootstrap.EnsureSchema(dataSource); + using var conn = dataSource.OpenConnection(); + var sql = limit is > 0 + ? """ + SELECT id, contract_instance_id, player_id, idempotency_key, + granted_items, granted_skill_xp, granted_reputation, recorded_at + FROM contract_outcome + WHERE contract_instance_id = @instance_id + ORDER BY recorded_at ASC, id ASC + LIMIT @lim; + """ + : """ + SELECT id, contract_instance_id, player_id, idempotency_key, + granted_items, granted_skill_xp, granted_reputation, recorded_at + FROM contract_outcome + WHERE contract_instance_id = @instance_id + ORDER BY recorded_at ASC, id ASC; + """; + using var cmd = new Npgsql.NpgsqlCommand(sql, conn); + cmd.Parameters.AddWithValue("instance_id", instanceId); + if (limit is > 0) + { + cmd.Parameters.AddWithValue("lim", limit.Value); + } + + var rows = new List(); + using var reader = cmd.ExecuteReader(); + while (reader.Read()) + { + rows.Add(ReadRow(reader)); + } + + return rows; + } + + /// + public bool TryClearForInstance(string contractInstanceId) + { + var instanceId = ContractInstanceIds.NormalizeContractInstanceId(contractInstanceId); + if (instanceId.Length == 0) + { + return false; + } + + PostgresContractOutcomeBootstrap.EnsureSchema(dataSource); + using var conn = dataSource.OpenConnection(); + using var cmd = new Npgsql.NpgsqlCommand( + """ + DELETE FROM contract_outcome + WHERE contract_instance_id = @instance_id; + """, + conn); + cmd.Parameters.AddWithValue("instance_id", instanceId); + cmd.ExecuteNonQuery(); + return true; + } + + private static ContractOutcomeRow ReadRow(Npgsql.NpgsqlDataReader reader) => + new( + reader.GetString(0), + reader.GetString(1), + reader.GetString(2), + reader.GetString(3), + DeserializeItems(reader.GetFieldValue(4)), + DeserializeSkillXp(reader.GetFieldValue(5)), + DeserializeReputation(reader.GetFieldValue(6)), + reader.GetFieldValue(7)); + + private static string SerializeGrants(IReadOnlyList grants) => + JsonSerializer.Serialize(grants, JsonOptions); + + private static IReadOnlyList DeserializeItems(string json) + { + var parsed = JsonSerializer.Deserialize(json, JsonOptions); + return parsed ?? []; + } + + private static IReadOnlyList DeserializeSkillXp(string json) + { + var parsed = JsonSerializer.Deserialize(json, JsonOptions); + return parsed ?? []; + } + + private static IReadOnlyList DeserializeReputation(string json) + { + var parsed = JsonSerializer.Deserialize(json, JsonOptions); + return parsed ?? []; + } + + private static bool TryNormalizeRow(ContractOutcomeRow row, out ContractOutcomeRow normalized) + { + normalized = null!; + var id = row.Id.Trim(); + var playerId = ContractInstanceIds.NormalizePlayerId(row.PlayerId); + var instanceId = ContractInstanceIds.NormalizeContractInstanceId(row.ContractInstanceId); + var idempotencyKey = row.IdempotencyKey.Trim(); + if (id.Length == 0 || + playerId.Length == 0 || + instanceId.Length == 0 || + idempotencyKey.Length == 0) + { + return false; + } + + normalized = row with + { + Id = id, + PlayerId = playerId, + ContractInstanceId = instanceId, + IdempotencyKey = idempotencyKey, + GrantedItems = row.GrantedItems.ToArray(), + GrantedSkillXp = row.GrantedSkillXp.ToArray(), + GrantedReputation = row.GrantedReputation.ToArray(), + }; + return true; + } +} diff --git a/server/NeonSprawl.Server/Program.cs b/server/NeonSprawl.Server/Program.cs index 0f6a688..c37a3f8 100644 --- a/server/NeonSprawl.Server/Program.cs +++ b/server/NeonSprawl.Server/Program.cs @@ -34,6 +34,7 @@ builder.Services.AddNpcBehaviorDefinitionCatalog(builder.Configuration); builder.Services.AddEncounterAndRewardCatalogs(builder.Configuration); builder.Services.AddQuestDefinitionCatalog(builder.Configuration); builder.Services.AddContractTemplateCatalog(builder.Configuration); +builder.Services.AddContractInstanceStores(builder.Configuration); builder.Services.AddPlayerQuestStateStore(builder.Configuration); builder.Services.AddRewardDeliveryStore(); builder.Services.AddFactionStandingStores(builder.Configuration); diff --git a/server/README.md b/server/README.md index f608cd1..c4e6282 100644 --- a/server/README.md +++ b/server/README.md @@ -269,6 +269,36 @@ Bundle- and gate-related schemas resolve as siblings under **`{parent of contrac On success, **Information** logs include the resolved contracts directory path, distinct template count, and catalog file count. Game code should use **`IContractTemplateRegistry`** for lookups (`TryGetDefinition`, `GetDefinitionsInIdOrder`; NEO-145). The catalog singleton remains for fail-fast startup only; do not inject **`ContractTemplateCatalog`** in new game code. +## Contract instance store (NEO-146) + +Per-player contract runtime state lives in **`IContractInstanceStore`**, keyed by **`contractInstanceId`** with at most **one active** row per player (prototype policy). Missing row ⇒ no instance; **`TryCreateActive`** creates an **`active`** row with template id, seed bucket, and issue timestamp. **`ContractGeneratorOperations`** (NEO-147) and **`ContractCompletionOperations`** (NEO-149) should call the store through orchestrators — not HTTP directly. + +**Store interface methods:** + +- **`CanWritePlayer`** — whether mutations are allowed for the player (dev bucket / Postgres `player_position`). +- **`TryGet`** — read one instance by player + instance id; false when absent. +- **`TryGetActiveForPlayer`** — read the player's single active instance, if any. +- **`TryCreateActive`** — first active for player returns `true`; denies when another active exists, duplicate instance id, or player not writable. +- **`TryMarkComplete`** — first completion returns `true`; replay returns `false` without changing **`completedAt`**. +- **`TryClearInstance`** — dev fixture only; removes one instance row. + +Completed instances are immutable (no status regression). Player, template, and instance ids are normalized (trim + lowercase for player/template/instance; seed bucket trim only). Template id validation against **`IContractTemplateRegistry`** is deferred to NEO-147. + +**Storage:** in-memory singleton when **`ConnectionStrings:NeonSprawl`** is unset (seeds configured dev player only). When Postgres is configured, **`PostgresContractInstanceStore`** persists to **`contract_instance`** ([`V011__contract_instance.sql`](../db/migrations/V011__contract_instance.sql)) with a partial unique index enforcing one active row per player. Plan: [NEO-146 implementation plan](../../docs/plans/NEO-146-implementation-plan.md). + +## Contract outcome store (NEO-146) + +Append-only contract completion audit rows live in **`IContractOutcomeStore`**. Each **`ContractOutcomeRow`** captures instance id, delivery idempotency key **`{playerId}:contract_complete:{contractInstanceId}`**, grant snapshot summary (items, skill XP, reputation — same shapes as **`RewardDeliveryEvent`**), and **`recordedAt`**. + +**Store interface methods:** + +- **`TryAppend`** — first row id or idempotency key returns `true`; duplicates return `false`. +- **`TryGetByIdempotencyKey`** — read one outcome for delivery replay / audit tests. +- **`GetOutcomesForInstance`** — list outcomes for one contract instance id (ordered by recorded time). +- **`TryClearForInstance`** — dev fixture only. + +**Storage:** in-memory when Postgres is unset; **`PostgresContractOutcomeStore`** appends to **`contract_outcome`** ([`V012__contract_outcome.sql`](../db/migrations/V012__contract_outcome.sql)) with JSONB grant snapshots when configured. Plan: [NEO-146 implementation plan](../../docs/plans/NEO-146-implementation-plan.md). + ## Quest definitions (NEO-115) **`GET /game/world/quest-definitions`** returns a versioned JSON body (`schemaVersion` **1**, **`quests`**) backed by **`IQuestDefinitionRegistry`** — the same prototype rows loaded at startup (no second source of truth). Each row includes **`id`**, **`displayName`**, **`prerequisiteQuestIds`**, optional **`factionGateRules`** (`factionId`, `minStanding` — omitted when the quest has no gates), and nested **`steps`** (`id`, `displayName`, **`objectives`** with flat objective fields per kind — unused keys omitted). Plan: [NEO-115 implementation plan](../../docs/plans/NEO-115-implementation-plan.md); gate projection: [NEO-140 implementation plan](../../docs/plans/NEO-140-implementation-plan.md); Bruno: `bruno/neon-sprawl-server/quest-definitions/`. diff --git a/server/db/migrations/V011__contract_instance.sql b/server/db/migrations/V011__contract_instance.sql new file mode 100644 index 0000000..b00c03e --- /dev/null +++ b/server/db/migrations/V011__contract_instance.sql @@ -0,0 +1,17 @@ +-- NEO-146: per-player contract instance rows (prototype E7.M4 Slice 4). +CREATE TABLE IF NOT EXISTS contract_instance ( + contract_instance_id TEXT NOT NULL PRIMARY KEY, + player_id TEXT NOT NULL REFERENCES player_position(player_id) ON DELETE CASCADE, + template_id TEXT NOT NULL, + status TEXT NOT NULL CHECK (status IN ('active', 'completed', 'expired')), + seed_bucket TEXT NOT NULL, + issued_at TIMESTAMPTZ NOT NULL, + completed_at TIMESTAMPTZ, + updated_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +CREATE UNIQUE INDEX IF NOT EXISTS idx_contract_instance_one_active_per_player + ON contract_instance (player_id) + WHERE status = 'active'; + +COMMENT ON TABLE contract_instance IS 'Persisted contract instances per player (NEO-146); at most one active row per player.'; diff --git a/server/db/migrations/V012__contract_outcome.sql b/server/db/migrations/V012__contract_outcome.sql new file mode 100644 index 0000000..b3db53e --- /dev/null +++ b/server/db/migrations/V012__contract_outcome.sql @@ -0,0 +1,17 @@ +-- NEO-146: append-only contract completion outcome audit log. +CREATE TABLE IF NOT EXISTS contract_outcome ( + id TEXT NOT NULL PRIMARY KEY, + contract_instance_id TEXT NOT NULL REFERENCES contract_instance(contract_instance_id) ON DELETE CASCADE, + player_id TEXT NOT NULL REFERENCES player_position(player_id) ON DELETE CASCADE, + idempotency_key TEXT NOT NULL UNIQUE, + granted_items JSONB NOT NULL DEFAULT '[]'::jsonb, + granted_skill_xp JSONB NOT NULL DEFAULT '[]'::jsonb, + granted_reputation JSONB NOT NULL DEFAULT '[]'::jsonb, + recorded_at TIMESTAMPTZ NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +CREATE INDEX IF NOT EXISTS idx_contract_outcome_instance_recorded + ON contract_outcome (contract_instance_id, recorded_at, id); + +COMMENT ON TABLE contract_outcome IS 'Append-only contract completion outcome audit (NEO-146).'; From be53d80705f3b2bd507cb35769edf15255a4b32b Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 24 Jun 2026 20:35:16 -0400 Subject: [PATCH 03/18] NEO-146: add code review for contract instance and outcome stores --- docs/reviews/2026-06-24-NEO-146.md | 70 ++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 docs/reviews/2026-06-24-NEO-146.md diff --git a/docs/reviews/2026-06-24-NEO-146.md b/docs/reviews/2026-06-24-NEO-146.md new file mode 100644 index 0000000..4727c1d --- /dev/null +++ b/docs/reviews/2026-06-24-NEO-146.md @@ -0,0 +1,70 @@ +# Code review — NEO-146 (E7M4-03 contract instance + outcome stores) + +**Date:** 2026-06-24 +**Scope:** Branch `NEO-146-e7m4-03-contract-instance-outcome-stores` — commits `82c4594`…`ae641a4` (2 commits) +**Base:** `main` + +## Verdict + +**Approve with nits** + +## Summary + +NEO-146 adds durable per-player contract instance state (`IContractInstanceStore`) and append-only completion audit rows (`IContractOutcomeStore`) with in-memory fallback and Postgres persistence (`V011`, `V012`) when `ConnectionStrings:NeonSprawl` is set. The implementation mirrors NEO-116/NEO-135 patterns: dev-player write gate, one-active enforcement (app logic + partial unique index), idempotent complete replay, outcome idempotency keys, grant snapshot reuse from `Game/Rewards/`, DI via `AddContractInstanceStores`, and test factory overrides. Unit tests cover happy/deny/idempotent paths with full AAA; Postgres integration tests include cross-factory persistence and a concurrent create race. `dotnet test` passes 862 tests. Risk is low — infrastructure-only, no HTTP or player-facing surface; primary consumers are NEO-147/NEO-149 orchestrators. + +## Documentation checked + +| Document | Alignment | +|----------|-----------| +| `docs/plans/NEO-146-implementation-plan.md` | **Matches** — AC checklist complete; reconciliation reflects shipped types, stores, migrations, DI, tests, README. Minor drift: plan lists `MakeActivePlayerKey` (not implemented) and defers Postgres `TryClear*` (implemented anyway — acceptable improvement). | +| `docs/plans/E7M4-pre-production-backlog.md` (E7M4-03) | **Matches** — dual stores, one-active policy, idempotency key shape, grant snapshots, NEO-116-style Postgres split; generator/payout/HTTP correctly out of scope. | +| `docs/decomposition/modules/E7_M4_ContractMissionGenerator.md` | **Matches** — runtime stores paragraph + README links added (NEO-146). | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Partially matches** — E7.M4 row cites E7M4-01/NEO-144 and E7M4-02/NEO-145 only; should add E7M4-03/NEO-146 stores after merge (same pattern as NEO-145 follow-up). | +| `docs/decomposition/modules/module_dependency_register.md` | **Partially matches** — E7.M4 In Progress spine unchanged; optional one-line NEO-146 note after merge. | +| `docs/decomposition/modules/client_server_authority.md` | **N/A** — server-side persistence only; no authority boundary change. | +| Full-stack epic decomposition | **N/A** — infrastructure-only; client counterpart correctly none (NEO-154 capstone). | + +Register/tracking tables should be updated for E7M4-03 after merge — **should fix** (non-blocking), mirroring NEO-145 alignment follow-up. + +## Blocking issues + +None. + +## Suggestions + +1. **Update decomposition tracking for E7M4-03** — After merge, extend `documentation_and_implementation_alignment.md` E7.M4 row (and optionally `module_dependency_register.md`) with E7M4-03: `IContractInstanceStore` + `IContractOutcomeStore`, `V011`/`V012`, README sections — same pattern as NEO-145. + +2. **Postgres one-active concurrent race** — The existing `TryCreateActive_ShouldReturnOneTrueAndRestFalse_WhenCalledConcurrently` test races on the **same** instance id (PK dedup), matching the quest-store precedent. For contracts, the more important race is **different** instance ids for the same player competing for the one-active partial unique index. Consider a second `RequirePostgresFact` that launches concurrent `TryCreateActive` with distinct instance ids and asserts exactly one `active` row remains. + +3. **Postgres outcome deny paths** — In-memory tests cover duplicate row id / idempotency key; Postgres integration only covers happy-path append. Add `RequirePostgresFact` duplicate-append deny (and optionally idempotent complete replay on Postgres) to lock SQL `INSERT … WHERE NOT EXISTS` / complete `FOR UPDATE` behavior. + +4. **In-memory duplicate instance id** — Plan §7 lists duplicate instance id deny; no explicit unit test calls `TryCreateActive` twice with the same instance id (second call should return `false` with unchanged snapshot). Cheap coverage gap. + +5. **Postgres outcome FK parity** — `PostgresContractOutcomeStore.TryAppend` will throw on FK violation (missing `contract_instance` row) while in-memory accepts any normalized row. Orchestrators (NEO-149) will create the instance first, so this is low risk today; document in README or catch `PostgresException` and return `false` if you want strict bool-only store semantics across backends. + +## Nits + +- Nit: `ContractInstanceIds.MakeInstanceKey` is defined but unused; plan also lists `MakeActivePlayerKey` which was not added. Either wire keys into store indexing or trim plan/reconciliation to match shipped API. + +- Nit: `ContractInstanceReasonCodes` is defined for E7M4-04+ orchestrators but unused in this slice — correct deferral; no action until NEO-147. + +- Nit: Bruno folder `bruno/neon-sprawl-server/contract-stores/` (health smoke) is useful but not listed in plan §Implementation reconciliation — optional one-line note for doc parity (same as NEO-145 Bruno nit). + +- Nit: Plan open questions marked Postgres `TryClear*` as deferred; both Postgres stores implement clear helpers anyway — update plan risk row to `adopted` on next doc pass. + +## Verification + +Commands run during review (all green): + +```bash +dotnet test NeonSprawl.sln +``` + +Author should run Postgres integration locally when `ConnectionStrings__NeonSprawl` is available: + +```bash +# Requires local Postgres + connection string +dotnet test NeonSprawl.sln --filter "FullyQualifiedName~ContractInstancePersistence|FullyQualifiedName~ContractOutcomePersistence" +``` + +Before merge: skim decomposition alignment suggestion — functional code is merge-ready without it. From 19229004770c02fe452150627e31f97733904d53 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 24 Jun 2026 20:37:16 -0400 Subject: [PATCH 04/18] NEO-146: address code review findings for contract stores Add Postgres race/deny tests, duplicate instance id coverage, FK-safe outcome append, MakeInstanceKey locking, decomposition alignment updates, and strike-through review doc. --- ...umentation_and_implementation_alignment.md | 2 +- .../modules/module_dependency_register.md | 2 +- docs/plans/NEO-146-implementation-plan.md | 7 +- docs/reviews/2026-06-24-NEO-146.md | 24 +++---- ...ractInstancePersistenceIntegrationTests.cs | 64 +++++++++++++++++++ ...tractOutcomePersistenceIntegrationTests.cs | 63 ++++++++++++++++++ .../InMemoryContractInstanceStoreTests.cs | 27 ++++++++ .../InMemoryContractInstanceStore.cs | 30 +++++++-- .../Contracts/PostgresContractOutcomeStore.cs | 9 ++- server/README.md | 2 +- 10 files changed, 206 insertions(+), 24 deletions(-) diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index 7da445a..6543e94 100644 --- a/docs/decomposition/modules/documentation_and_implementation_alignment.md +++ b/docs/decomposition/modules/documentation_and_implementation_alignment.md @@ -62,7 +62,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not | E7.M2 | Ready | **E7M2-01 catalog landed ([NEO-124](https://linear.app/neon-sprawl/issue/NEO-124)):** `quest-reward-bundle` + `quest-skill-xp-grant` schemas; **`completionRewardBundle`** on four frozen E7.M1 quests; CI bundle freeze + cross-ref gates. **E7M2-02 server load ([NEO-125](https://linear.app/neon-sprawl/issue/NEO-125)):** fail-fast quest catalog **`completionRewardBundle`** validation at startup — `PrototypeE7M2QuestCatalogRules` + bundle schema `$ref` registration ([NEO-125](../../plans/NEO-125-implementation-plan.md)); [server README — Quest catalog (NEO-125)](../../../server/README.md#quest-catalog-contentquests-neo-113). **E7M2-03 delivery store ([NEO-126](https://linear.app/neon-sprawl/issue/NEO-126)):** idempotent **`IRewardDeliveryStore`** + **`RewardDeliveryEvent`** in `Game/Rewards/` — in-memory prototype ([NEO-126](../../plans/NEO-126-implementation-plan.md)); [server README — Reward delivery store (NEO-126)](../../../server/README.md#reward-delivery-store-neo-126). **E7M2-04 router apply ([NEO-127](https://linear.app/neon-sprawl/issue/NEO-127)):** **`RewardRouterOperations.TryDeliverQuestCompletion`** — item + skill XP bundle apply with compensating rollback + store record ([NEO-127](../../plans/NEO-127-implementation-plan.md)); [server README — Reward router (NEO-127)](../../../server/README.md#reward-router-neo-127). **E7M2-05 quest-state wiring ([NEO-128](https://linear.app/neon-sprawl/issue/NEO-128)):** **`QuestStateOperations.TryMarkComplete`** + **`QuestObjectiveWiring`** deliver-then-mark via router ([NEO-128](../../plans/NEO-128-implementation-plan.md)); [server README — Quest state operations (NEO-117)](../../../server/README.md#quest-state-operations-neo-117). **E7M2-06 HTTP read ([NEO-129](https://linear.app/neon-sprawl/issue/NEO-129)):** **`GET …/quest-progress`** **`completionRewardSummary`** from **`IRewardDeliveryStore`** ([NEO-129](../../plans/NEO-129-implementation-plan.md)); Bruno `bruno/neon-sprawl-server/quest-progress/Get quest progress after gather intro complete.bru`. **E7M2-07 telemetry ([NEO-130](https://linear.app/neon-sprawl/issue/NEO-130)):** comment-only **`reward_delivery`** + **`unlock_granted`** stub hook sites in **`RewardRouterOperations.TryDeliverQuestCompletion`** ([NEO-130](../../plans/NEO-130-implementation-plan.md)); [server README — Reward telemetry hooks (NEO-130)](../../../server/README.md#reward-telemetry-hooks-neo-130). **E7M2-08 client HUD ([NEO-131](https://linear.app/neon-sprawl/issue/NEO-131)):** Godot **`QuestRewardDeliveryLabel`** paints **`completionRewardSummary`** on in-session completion transition ([NEO-131](../../plans/NEO-131-implementation-plan.md)); [client README — Quest completion reward HUD (NEO-131)](../../../client/README.md#quest-completion-reward-hud-neo-131). **E7M2-09 client capstone ([NEO-132](https://linear.app/neon-sprawl/issue/NEO-132)):** playable quest reward delivery loop — [`NEO-132` manual QA](../../manual-qa/NEO-132.md); [client README — End-to-end quest reward loop (NEO-132)](../../../client/README.md#end-to-end-quest-reward-loop-neo-132); plan [NEO-132](../../plans/NEO-132-implementation-plan.md). **Epic 7 Slice 2 client capstone complete.** Backlog **E7M2-01** [NEO-124](https://linear.app/neon-sprawl/issue/NEO-124) → **E7M2-09** [NEO-132](https://linear.app/neon-sprawl/issue/NEO-132) **landed**. **NEO-43 prep landed:** **`MissionRewardSkillXpGrant`**. **Encounter loot unchanged** (E5.M3 direct grants). Upstream: E7.M1 **Ready**, E2.M2 grant stack, E3.M3 inventory **Ready**. | [E7M2-prototype-backlog.md](../../plans/E7M2-prototype-backlog.md), [E7_M2](E7_M2_RewardAndUnlockRouter.md), [NEO-124](../../plans/NEO-124-implementation-plan.md), [NEO-125](../../plans/NEO-125-implementation-plan.md), [NEO-126](../../plans/NEO-126-implementation-plan.md), [NEO-127](../../plans/NEO-127-implementation-plan.md), [NEO-128](../../plans/NEO-128-implementation-plan.md), [NEO-129](../../plans/NEO-129-implementation-plan.md), [NEO-130](../../plans/NEO-130-implementation-plan.md), [NEO-131](../../plans/NEO-131-implementation-plan.md), [NEO-132](../../plans/NEO-132-implementation-plan.md), [NEO-43](../../plans/NEO-43-implementation-plan.md), label **`E7.M2`** on NEO-124–NEO-132 | | E7.M1 | Ready | **E7M1-01 catalog landed ([NEO-112](https://linear.app/neon-sprawl/issue/NEO-112)):** quest schemas, `prototype_quests.json`, CI gates (four frozen quest ids, objective cross-refs, acyclic prerequisites, chain terminal token). **E7M1-02 server load ([NEO-113](https://linear.app/neon-sprawl/issue/NEO-113)):** fail-fast startup load of `content/quests/*_quests.json` — `server/NeonSprawl.Server/Game/Quests/` ([NEO-113](../../plans/NEO-113-implementation-plan.md)); [server README — Quest catalog](../../../server/README.md#quest-catalog-contentquests-neo-113). **E7M1-03 registry ([NEO-114](https://linear.app/neon-sprawl/issue/NEO-114)):** **`IQuestDefinitionRegistry`** + DI ([NEO-114](../../plans/NEO-114-implementation-plan.md)). **E7M1-04 HTTP read ([NEO-115](https://linear.app/neon-sprawl/issue/NEO-115)):** **`GET /game/world/quest-definitions`** — `QuestDefinitionsWorldApi` + DTOs ([NEO-115](../../plans/NEO-115-implementation-plan.md)); [server README — Quest definitions (NEO-115)](../../../server/README.md#quest-definitions-neo-115); Bruno `bruno/neon-sprawl-server/quest-definitions/`. **E7M1-05 store ([NEO-116](https://linear.app/neon-sprawl/issue/NEO-116)):** **`IPlayerQuestStateStore`** + in-memory/Postgres persistence ([NEO-116](../../plans/NEO-116-implementation-plan.md)); [server README — Quest progress store (NEO-116)](../../../server/README.md#quest-progress-store-neo-116). **E7M1-06 operations ([NEO-117](https://linear.app/neon-sprawl/issue/NEO-117)):** **`QuestStateOperations`** + reason codes ([NEO-117](../../plans/NEO-117-implementation-plan.md)); [server README — Quest state operations (NEO-117)](../../../server/README.md#quest-state-operations-neo-117). **E7M1-07 objective wiring ([NEO-118](https://linear.app/neon-sprawl/issue/NEO-118)):** **`QuestObjectiveWiring`** on gather/craft/encounter + **`inventory_has_item`** snapshot passes ([NEO-118](../../plans/NEO-118-implementation-plan.md)); [server README — Quest objective wiring (NEO-118)](../../../server/README.md#quest-objective-wiring-neo-118). **E7M1-08 per-player GET ([NEO-119](https://linear.app/neon-sprawl/issue/NEO-119)):** **`GET /game/players/{id}/quest-progress`** — `QuestProgressApi` + DTOs; GET-side inventory refresh ([NEO-119](../../plans/NEO-119-implementation-plan.md)); [server README — Per-player quest progress (NEO-119)](../../../server/README.md#per-player-quest-progress-neo-119); Bruno `bruno/neon-sprawl-server/quest-progress/`. **E7M1-09 accept POST ([NEO-120](https://linear.app/neon-sprawl/issue/NEO-120)):** **`POST /game/players/{id}/quests/{questId}/accept`** — `QuestAcceptApi` + DTOs; **`QuestStateOperations.TryAccept`** ([NEO-120](../../plans/NEO-120-implementation-plan.md)); [server README — Quest accept POST (NEO-120)](../../../server/README.md#quest-accept-post-neo-120); Bruno accept spine in `bruno/neon-sprawl-server/quest-progress/`. **E7M1-10 telemetry ([NEO-121](https://linear.app/neon-sprawl/issue/NEO-121)):** comment-only **`quest_start`**, **`quest_step_complete`**, **`quest_complete`**, **`quest_accept_denied`** hook sites in **`QuestStateOperations`** ([NEO-121](../../plans/NEO-121-implementation-plan.md)); [server README — Quest telemetry hooks (NEO-121)](../../../server/README.md#quest-telemetry-hooks-neo-121). **E7M1-11 client HUD ([NEO-122](https://linear.app/neon-sprawl/issue/NEO-122)):** **`quest_progress_client.gd`**, **`quest_definitions_client.gd`**, **`QuestProgressLabel`** / **`QuestAcceptFeedbackLabel`**; boot + event-driven GET refresh; **Q** / **Shift+Q** accept ([NEO-122](../../plans/NEO-122-implementation-plan.md), [`NEO-122` manual QA](../../manual-qa/NEO-122.md)); [client README — Quest progress + accept HUD (NEO-122)](../../../client/README.md#quest-progress--accept-hud-neo-122). **E7M1-12 client capstone ([NEO-123](https://linear.app/neon-sprawl/issue/NEO-123)):** playable four-quest onboarding chain — [`NEO-123` manual QA](../../manual-qa/NEO-123.md); [client README — End-to-end onboarding quest loop (NEO-123)](../../../client/README.md#end-to-end-onboarding-quest-loop-neo-123); plan [NEO-123](../../plans/NEO-123-implementation-plan.md). **Epic 7 Slice 1 client capstone complete.** Backlog **E7M1-01** [NEO-112](https://linear.app/neon-sprawl/issue/NEO-112) → **E7M1-12** [NEO-123](https://linear.app/neon-sprawl/issue/NEO-123) **landed**. Upstream: E3.M1 gather **Ready**, E3.M2 craft **Ready**, E5.M3 encounter **`contract_handoff_token`** + **`EncounterCompleteEvent`** **Ready**. | [NEO-123 plan](../../plans/NEO-123-implementation-plan.md), [NEO-122 plan](../../plans/NEO-122-implementation-plan.md), [NEO-121 plan](../../plans/NEO-121-implementation-plan.md), [NEO-120 plan](../../plans/NEO-120-implementation-plan.md), [NEO-119 plan](../../plans/NEO-119-implementation-plan.md), [NEO-118 plan](../../plans/NEO-118-implementation-plan.md), [NEO-117 plan](../../plans/NEO-117-implementation-plan.md), [NEO-116 plan](../../plans/NEO-116-implementation-plan.md), [NEO-115 plan](../../plans/NEO-115-implementation-plan.md), [NEO-114 plan](../../plans/NEO-114-implementation-plan.md), [NEO-113 plan](../../plans/NEO-113-implementation-plan.md), [NEO-112 plan](../../plans/NEO-112-implementation-plan.md), [E7M1-prototype-backlog.md](../../plans/E7M1-prototype-backlog.md), [E7_M1](E7_M1_QuestStateMachine.md), label **`E7.M1`** on NEO-112–NEO-123 | | E7.M3 | Ready | **E7M3-01 catalog landed ([NEO-133](https://linear.app/neon-sprawl/issue/NEO-133)):** faction + gate/rep schemas, `prototype_factions.json`, quest extensions, five-quest CI gates, minimal server roster parity. **E7M3-02 server load landed ([NEO-134](https://linear.app/neon-sprawl/issue/NEO-134)):** fail-fast faction catalog, `IFactionDefinitionRegistry`, quest `factionGateRules` / `reputationGrants` parse + E7M3 cross-ref/bundle/grid gates ([NEO-134 plan](../../plans/NEO-134-implementation-plan.md)). **E7M3-03 stores landed ([NEO-135](https://linear.app/neon-sprawl/issue/NEO-135)):** `IFactionStandingStore`, `IReputationDeltaStore`, in-memory + Postgres `V009`/`V010` ([NEO-135 plan](../../plans/NEO-135-implementation-plan.md)); [server README — Faction standing store (NEO-135)](../../../server/README.md#faction-standing-store-neo-135). **E7M3-04 ops landed ([NEO-136](https://linear.app/neon-sprawl/issue/NEO-136)):** `ReputationOperations.TryApplyDelta` — standing + audit orchestration with compensating rollback ([NEO-136 plan](../../plans/NEO-136-implementation-plan.md)); [server README — ReputationOperations (NEO-136)](../../../server/README.md#reputationoperations-neo-136). **E7M3-05 gate eval landed ([NEO-137](https://linear.app/neon-sprawl/issue/NEO-137)):** `FactionGateOperations.TryEvaluate` wired into `QuestStateOperations.TryAccept`; `faction_gate_blocked` deny ([NEO-137 plan](../../plans/NEO-137-implementation-plan.md)); [server README — FactionGateOperations (NEO-137)](../../../server/README.md#factiongateoperations-neo-137). **E7M3-06 reward router landed ([NEO-138](https://linear.app/neon-sprawl/issue/NEO-138)):** `RewardRouterOperations` applies `reputationGrants` via `ReputationOperations`; Bruno grid-contract accept success after organic operator chain ([NEO-138 plan](../../plans/NEO-138-implementation-plan.md)); [server README — Reward router (NEO-127)](../../../server/README.md#reward-router-neo-127). **E7M3-07 HTTP read landed ([NEO-139](https://linear.app/neon-sprawl/issue/NEO-139)):** **`GET …/faction-standing`** — `FactionStandingApi` + DTOs; Bruno `bruno/neon-sprawl-server/faction-standing/` ([NEO-139 plan](../../plans/NEO-139-implementation-plan.md)); [server README — Faction standing read (NEO-139)](../../../server/README.md#faction-standing-read-neo-139). **E7M3-08 quest HTTP projections landed ([NEO-140](https://linear.app/neon-sprawl/issue/NEO-140)):** world GET **`factionGateRules`** + quest-progress **`completionRewardSummary.reputationGrants`**; Bruno quest-definitions + operator-chain progress bru ([NEO-140 plan](../../plans/NEO-140-implementation-plan.md)); [server README — Quest definitions (NEO-115)](../../../server/README.md#quest-definitions-neo-115), [Per-player quest progress (NEO-119)](../../../server/README.md#per-player-quest-progress-neo-119). **E7M3-09 telemetry landed ([NEO-141](https://linear.app/neon-sprawl/issue/NEO-141)):** comment-only **`reputation_delta`** + **`faction_gate_blocked`** hook sites in ops layers ([NEO-141 plan](../../plans/NEO-141-implementation-plan.md)); [server README — Faction telemetry hooks (NEO-141)](../../../server/README.md#faction-telemetry-hooks-neo-141). **E7M3-10 client faction HUD landed ([NEO-142](https://linear.app/neon-sprawl/issue/NEO-142)):** `faction_standing_client.gd`, standing label + gate deny + rep reward lines ([NEO-142 plan](../../plans/NEO-142-implementation-plan.md)); [client README — Faction standing + gate feedback HUD (NEO-142)](../../../client/README.md#faction-standing--gate-feedback-hud-neo-142); [`NEO-142` manual QA](../../manual-qa/NEO-142.md). **E7M3-11 client capstone landed ([NEO-143](https://linear.app/neon-sprawl/issue/NEO-143)):** playable faction reputation + gate capstone — [`NEO-143` manual QA](../../manual-qa/NEO-143.md); [client README — End-to-end faction reputation loop (NEO-143)](../../../client/README.md#end-to-end-faction-reputation-loop-neo-143); plan [NEO-143](../../plans/NEO-143-implementation-plan.md). **Epic 7 Slice 3 client capstone complete.** Two frozen factions; operator-chain **`reputationGrants`** (+15 Grid Operators); gated **`prototype_quest_grid_contract`**. Upstream: E7.M1 **Ready**, E7.M2 **Ready**. | [NEO-133 plan](../../plans/NEO-133-implementation-plan.md), [NEO-134 plan](../../plans/NEO-134-implementation-plan.md), [NEO-135 plan](../../plans/NEO-135-implementation-plan.md), [NEO-136 plan](../../plans/NEO-136-implementation-plan.md), [NEO-137 plan](../../plans/NEO-137-implementation-plan.md), [NEO-138 plan](../../plans/NEO-138-implementation-plan.md), [NEO-139 plan](../../plans/NEO-139-implementation-plan.md), [NEO-140 plan](../../plans/NEO-140-implementation-plan.md), [NEO-141 plan](../../plans/NEO-141-implementation-plan.md), [NEO-142 plan](../../plans/NEO-142-implementation-plan.md), [NEO-143 plan](../../plans/NEO-143-implementation-plan.md), [E7M3-pre-production-backlog.md](../../plans/E7M3-pre-production-backlog.md), [E7_M3](E7_M3_FactionReputationLedger.md), label **`E7.M3`** on NEO-133–NEO-143 | -| E7.M4 | In Progress | **E7M4-01 catalog landed ([NEO-144](https://linear.app/neon-sprawl/issue/NEO-144)):** `contract-template` + `contract-seed` schemas; `prototype_contract_templates.json`; CI roster/freeze/cross-ref/band-cap gates. **E7M4-02 server load landed ([NEO-145](https://linear.app/neon-sprawl/issue/NEO-145)):** fail-fast contract template catalog, `IContractTemplateRegistry`, `PrototypeE7M4ContractCatalogRules` at startup ([NEO-145 plan](../../plans/NEO-145-implementation-plan.md)); [server README — Contract template catalog](../../../server/README.md#contract-template-catalog-contentcontracts-neo-145). **Decomposition landed:** Epic 7 Slice 4 backlog [E7M4-pre-production-backlog.md](../../plans/E7M4-pre-production-backlog.md) — **E7M4-01** [NEO-144](https://linear.app/neon-sprawl/issue/NEO-144) → **E7M4-11** [NEO-154](https://linear.app/neon-sprawl/issue/NEO-154); label **`E7.M4`**. Prototype spine: **`prototype_contract_clear_combat_pocket`** template → **`prototype_combat_pocket`** encounter clear → repeat payout **`scrap_metal_bulk` ×5** + salvage **15**; one active contract per player; economy band caps at issue; E4.M1 live zone deferred (content **`zoneDifficultyBand`**). Client capstone **E7M4-11** [NEO-154](https://linear.app/neon-sprawl/issue/NEO-154). Upstream: E5.M3 **Ready**, E7.M2 router **Ready**, E7.M3 standing **Ready**. | [E7M4-pre-production-backlog.md](../../plans/E7M4-pre-production-backlog.md), [E7_M4](E7_M4_ContractMissionGenerator.md), [NEO-144](../../plans/NEO-144-implementation-plan.md), [NEO-145](../../plans/NEO-145-implementation-plan.md), label **`E7.M4`** on NEO-144–NEO-154 | +| E7.M4 | In Progress | **E7M4-01 catalog landed ([NEO-144](https://linear.app/neon-sprawl/issue/NEO-144)):** `contract-template` + `contract-seed` schemas; `prototype_contract_templates.json`; CI roster/freeze/cross-ref/band-cap gates. **E7M4-02 server load landed ([NEO-145](https://linear.app/neon-sprawl/issue/NEO-145)):** fail-fast contract template catalog, `IContractTemplateRegistry`, `PrototypeE7M4ContractCatalogRules` at startup ([NEO-145 plan](../../plans/NEO-145-implementation-plan.md)); [server README — Contract template catalog](../../../server/README.md#contract-template-catalog-contentcontracts-neo-145). **E7M4-03 stores landed ([NEO-146](https://linear.app/neon-sprawl/issue/NEO-146)):** `IContractInstanceStore`, `IContractOutcomeStore`, in-memory + Postgres `V011`/`V012` ([NEO-146 plan](../../plans/NEO-146-implementation-plan.md)); [server README — Contract instance store (NEO-146)](../../../server/README.md#contract-instance-store-neo-146), [Contract outcome store (NEO-146)](../../../server/README.md#contract-outcome-store-neo-146); Bruno `bruno/neon-sprawl-server/contract-stores/`. **Decomposition landed:** Epic 7 Slice 4 backlog [E7M4-pre-production-backlog.md](../../plans/E7M4-pre-production-backlog.md) — **E7M4-01** [NEO-144](https://linear.app/neon-sprawl/issue/NEO-144) → **E7M4-11** [NEO-154](https://linear.app/neon-sprawl/issue/NEO-154); label **`E7.M4`**. Prototype spine: **`prototype_contract_clear_combat_pocket`** template → **`prototype_combat_pocket`** encounter clear → repeat payout **`scrap_metal_bulk` ×5** + salvage **15**; one active contract per player; economy band caps at issue; E4.M1 live zone deferred (content **`zoneDifficultyBand`**). Client capstone **E7M4-11** [NEO-154](https://linear.app/neon-sprawl/issue/NEO-154). Upstream: E5.M3 **Ready**, E7.M2 router **Ready**, E7.M3 standing **Ready**. | [E7M4-pre-production-backlog.md](../../plans/E7M4-pre-production-backlog.md), [E7_M4](E7_M4_ContractMissionGenerator.md), [NEO-144](../../plans/NEO-144-implementation-plan.md), [NEO-145](../../plans/NEO-145-implementation-plan.md), [NEO-146](../../plans/NEO-146-implementation-plan.md), label **`E7.M4`** on NEO-144–NEO-154 | --- diff --git a/docs/decomposition/modules/module_dependency_register.md b/docs/decomposition/modules/module_dependency_register.md index 8aeee37..32b2806 100644 --- a/docs/decomposition/modules/module_dependency_register.md +++ b/docs/decomposition/modules/module_dependency_register.md @@ -118,7 +118,7 @@ Gameplay **content and curves** default to **boot load** with optional **dev-onl **E7.M3 note:** Epic 7 **Slice 3** backlog in Linear ([Epic 7 — Questing, Narrative, and Faction Reputation](https://linear.app/neon-sprawl/project/epic-7-questing-narrative-and-faction-reputation-a9416783ceee)): [NEO-133](https://linear.app/neon-sprawl/issue/NEO-133) → [NEO-143](https://linear.app/neon-sprawl/issue/NEO-143) **landed**; label **`E7.M3`**. See [E7M3-pre-production-backlog.md](../../plans/E7M3-pre-production-backlog.md), [E7_M3_FactionReputationLedger.md](E7_M3_FactionReputationLedger.md). Upstream: E7.M1 **Ready**, E7.M2 **Ready**. Prototype spine: two frozen factions; operator chain grants **+15 Grid Operators**; **`prototype_quest_grid_contract`** gated at **minStanding 15**; rep grants via **`completionRewardBundle.reputationGrants`**; accept deny **`faction_gate_blocked`**. **E7M3-11 / NEO-143** client capstone landed — [`NEO-143` manual QA](../../manual-qa/NEO-143.md), [client README — End-to-end faction reputation loop (NEO-143)](../../../client/README.md#end-to-end-faction-reputation-loop-neo-143), plan [NEO-143](../../plans/NEO-143-implementation-plan.md). Epic 7 Slice 3 client capstone complete. | E7.M4 | ContractMissionGenerator | E4.M1, E5.M3, E7.M3 | ContractTemplate, ContractSeed, ContractOutcome | Pre-production | In Progress | -**E7.M4 note:** Epic 7 **Slice 4** backlog in Linear ([Epic 7 — Questing, Narrative, and Faction Reputation](https://linear.app/neon-sprawl/project/epic-7-questing-narrative-and-faction-reputation-a9416783ceee)): [NEO-144](https://linear.app/neon-sprawl/issue/NEO-144) → [NEO-154](https://linear.app/neon-sprawl/issue/NEO-154); label **`E7.M4`**. See [E7M4-pre-production-backlog.md](../../plans/E7M4-pre-production-backlog.md), [E7_M4_ContractMissionGenerator.md](E7_M4_ContractMissionGenerator.md). **E7M4-01 / NEO-144** catalog + CI landed; **E7M4-02 / NEO-145** fail-fast server catalog load + **`IContractTemplateRegistry`** landed ([NEO-145 plan](../../plans/NEO-145-implementation-plan.md)). Upstream: E5.M3 **Ready**, E7.M3 **Ready**; E4.M1 live zone read deferred (content-only **`zoneDifficultyBand`** in Slice 4 v1). Prototype spine: one template **`prototype_contract_clear_combat_pocket`** → **`prototype_combat_pocket`** encounter clear → repeat **`scrap_metal_bulk` ×5** + salvage XP; economy band caps at issue. Client capstone **E7M4-11** [NEO-154](https://linear.app/neon-sprawl/issue/NEO-154). +**E7.M4 note:** Epic 7 **Slice 4** backlog in Linear ([Epic 7 — Questing, Narrative, and Faction Reputation](https://linear.app/neon-sprawl/project/epic-7-questing-narrative-and-faction-reputation-a9416783ceee)): [NEO-144](https://linear.app/neon-sprawl/issue/NEO-144) → [NEO-154](https://linear.app/neon-sprawl/issue/NEO-154); label **`E7.M4`**. See [E7M4-pre-production-backlog.md](../../plans/E7M4-pre-production-backlog.md), [E7_M4_ContractMissionGenerator.md](E7_M4_ContractMissionGenerator.md). **E7M4-01 / NEO-144** catalog + CI landed; **E7M4-02 / NEO-145** fail-fast server catalog load + **`IContractTemplateRegistry`** landed ([NEO-145 plan](../../plans/NEO-145-implementation-plan.md)); **E7M4-03 / NEO-146** contract instance + outcome stores landed ([NEO-146 plan](../../plans/NEO-146-implementation-plan.md)). Upstream: E5.M3 **Ready**, E7.M3 **Ready**; E4.M1 live zone read deferred (content-only **`zoneDifficultyBand`** in Slice 4 v1). Prototype spine: one template **`prototype_contract_clear_combat_pocket`** → **`prototype_combat_pocket`** encounter clear → repeat **`scrap_metal_bulk` ×5** + salvage XP; economy band caps at issue. Client capstone **E7M4-11** [NEO-154](https://linear.app/neon-sprawl/issue/NEO-154). ### Epic 8 — Social / Guild diff --git a/docs/plans/NEO-146-implementation-plan.md b/docs/plans/NEO-146-implementation-plan.md index 8bf5213..bba779f 100644 --- a/docs/plans/NEO-146-implementation-plan.md +++ b/docs/plans/NEO-146-implementation-plan.md @@ -68,8 +68,9 @@ Durable per-player **`ContractInstance`** rows (active/completed) and append-onl - **Types:** `ContractInstanceStatus`, `ContractInstanceState`, `ContractInstanceIds`, `ContractInstanceReasonCodes`, `ContractOutcomeRow`, `ContractOutcomeIds`. - **Stores:** `IContractInstanceStore` / `IContractOutcomeStore`; in-memory + Postgres (`V011`, `V012`) when connection string set; partial unique index + app-level one-active enforcement. - **DI:** `AddContractInstanceStores` in `Program.cs`; in-memory override in `InMemoryWebApplicationFactory`. -- **Tests:** `InMemoryContractInstanceStoreTests`, `InMemoryContractOutcomeStoreTests`, Postgres persistence integration tests; host DI smoke (`862` tests green). +- **Tests:** `InMemoryContractInstanceStoreTests`, `InMemoryContractOutcomeStoreTests`, Postgres persistence integration tests; host DI smoke (`866` tests green). - **Docs:** `server/README.md` contract instance + outcome store sections. +- **Bruno:** `bruno/neon-sprawl-server/contract-stores/` health smoke (startup-only; no contract HTTP in E7M4-03). ## Technical approach @@ -79,7 +80,7 @@ Durable per-player **`ContractInstance`** rows (active/completed) and append-onl |------|------| | `ContractInstanceStatus` | `Active`, `Completed`, `Expired` (no expiry transition in E7M4-03) | | `ContractInstanceState` | Immutable snapshot: `ContractInstanceId`, `TemplateId`, `PlayerId`, `Status`, `SeedBucket`, `IssuedAt`, `CompletedAt?` | -| `ContractInstanceIds` | `NormalizePlayerId`, `NormalizeContractInstanceId`, `MakeInstanceKey`, `MakeActivePlayerKey` | +| `ContractInstanceIds` | `NormalizePlayerId`, `NormalizeContractInstanceId`, `MakeInstanceKey` (per-row lock key) | | `ContractInstanceReasonCodes` | `player_not_writable`, `active_contract_exists`, `instance_not_found`, `instance_not_active`, `invalid_ids` | | `ContractOutcomeRow` | `Id`, `ContractInstanceId`, `PlayerId`, `IdempotencyKey`, grant snapshots (items/skill XP/rep), `RecordedAt` | | `ContractOutcomeIds` | `MakeIdempotencyKey(playerId, contractInstanceId)` → `{p}:contract_complete:{i}` | @@ -229,7 +230,7 @@ Manual Godot QA: **none** (server infrastructure; capstone is NEO-154). | Postgres partial unique index vs app-only one-active check | **Both** — enforce in store logic (fast deny reason) + partial unique index for race safety | `adopted` | | Grant snapshot on outcome before E7M4-05 router | Store accepts caller-supplied snapshot on append; E7M4-06 orchestrator fills from router result | `adopted` | | `Expired` status unused in Slice 4 | Keep enum for forward compatibility; no API until a future expiry story | `adopted` | -| Dev `TryClear*` on Postgres | Implement on in-memory only first; add Postgres clear if Bruno fixture story needs it (mirror quest `TryClearProgress` on both when needed) | `deferred` | +| Dev `TryClear*` on Postgres | Implement on in-memory only first; add Postgres clear if Bruno fixture story needs it (mirror quest `TryClearProgress` on both when needed) | `adopted` — both backends implement clear helpers | ## Client counterpart diff --git a/docs/reviews/2026-06-24-NEO-146.md b/docs/reviews/2026-06-24-NEO-146.md index 4727c1d..6acd7c8 100644 --- a/docs/reviews/2026-06-24-NEO-146.md +++ b/docs/reviews/2026-06-24-NEO-146.md @@ -19,12 +19,12 @@ NEO-146 adds durable per-player contract instance state (`IContractInstanceStore | `docs/plans/NEO-146-implementation-plan.md` | **Matches** — AC checklist complete; reconciliation reflects shipped types, stores, migrations, DI, tests, README. Minor drift: plan lists `MakeActivePlayerKey` (not implemented) and defers Postgres `TryClear*` (implemented anyway — acceptable improvement). | | `docs/plans/E7M4-pre-production-backlog.md` (E7M4-03) | **Matches** — dual stores, one-active policy, idempotency key shape, grant snapshots, NEO-116-style Postgres split; generator/payout/HTTP correctly out of scope. | | `docs/decomposition/modules/E7_M4_ContractMissionGenerator.md` | **Matches** — runtime stores paragraph + README links added (NEO-146). | -| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Partially matches** — E7.M4 row cites E7M4-01/NEO-144 and E7M4-02/NEO-145 only; should add E7M4-03/NEO-146 stores after merge (same pattern as NEO-145 follow-up). | -| `docs/decomposition/modules/module_dependency_register.md` | **Partially matches** — E7.M4 In Progress spine unchanged; optional one-line NEO-146 note after merge. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | ~~**Partially matches** — E7.M4 row cites E7M4-01/NEO-144 and E7M4-02/NEO-145 only; should add E7M4-03/NEO-146 stores after merge (same pattern as NEO-145 follow-up).~~ **Done.** — E7M4-03 / NEO-146 stores added to E7.M4 row. | +| `docs/decomposition/modules/module_dependency_register.md` | ~~**Partially matches** — E7.M4 In Progress spine unchanged; optional one-line NEO-146 note after merge.~~ **Done.** — E7M4-03 / NEO-146 note added. | | `docs/decomposition/modules/client_server_authority.md` | **N/A** — server-side persistence only; no authority boundary change. | | Full-stack epic decomposition | **N/A** — infrastructure-only; client counterpart correctly none (NEO-154 capstone). | -Register/tracking tables should be updated for E7M4-03 after merge — **should fix** (non-blocking), mirroring NEO-145 alignment follow-up. +~~Register/tracking tables should be updated for E7M4-03 after merge — **should fix** (non-blocking), mirroring NEO-145 alignment follow-up.~~ **Done.** ## Blocking issues @@ -32,25 +32,25 @@ None. ## Suggestions -1. **Update decomposition tracking for E7M4-03** — After merge, extend `documentation_and_implementation_alignment.md` E7.M4 row (and optionally `module_dependency_register.md`) with E7M4-03: `IContractInstanceStore` + `IContractOutcomeStore`, `V011`/`V012`, README sections — same pattern as NEO-145. +1. ~~**Update decomposition tracking for E7M4-03** — After merge, extend `documentation_and_implementation_alignment.md` E7.M4 row (and optionally `module_dependency_register.md`) with E7M4-03: `IContractInstanceStore` + `IContractOutcomeStore`, `V011`/`V012`, README sections — same pattern as NEO-145.~~ **Done.** -2. **Postgres one-active concurrent race** — The existing `TryCreateActive_ShouldReturnOneTrueAndRestFalse_WhenCalledConcurrently` test races on the **same** instance id (PK dedup), matching the quest-store precedent. For contracts, the more important race is **different** instance ids for the same player competing for the one-active partial unique index. Consider a second `RequirePostgresFact` that launches concurrent `TryCreateActive` with distinct instance ids and asserts exactly one `active` row remains. +2. ~~**Postgres one-active concurrent race** — The existing `TryCreateActive_ShouldReturnOneTrueAndRestFalse_WhenCalledConcurrently` test races on the **same** instance id (PK dedup), matching the quest-store precedent. For contracts, the more important race is **different** instance ids for the same player competing for the one-active partial unique index. Consider a second `RequirePostgresFact` that launches concurrent `TryCreateActive` with distinct instance ids and asserts exactly one `active` row remains.~~ **Done.** — `TryCreateActive_ShouldAllowOnlyOneActive_WhenDistinctInstanceIdsRace`. -3. **Postgres outcome deny paths** — In-memory tests cover duplicate row id / idempotency key; Postgres integration only covers happy-path append. Add `RequirePostgresFact` duplicate-append deny (and optionally idempotent complete replay on Postgres) to lock SQL `INSERT … WHERE NOT EXISTS` / complete `FOR UPDATE` behavior. +3. ~~**Postgres outcome deny paths** — In-memory tests cover duplicate row id / idempotency key; Postgres integration only covers happy-path append. Add `RequirePostgresFact` duplicate-append deny (and optionally idempotent complete replay on Postgres) to lock SQL `INSERT … WHERE NOT EXISTS` / complete `FOR UPDATE` behavior.~~ **Done.** — duplicate-append deny, missing-instance FK deny, and Postgres idempotent complete tests added. -4. **In-memory duplicate instance id** — Plan §7 lists duplicate instance id deny; no explicit unit test calls `TryCreateActive` twice with the same instance id (second call should return `false` with unchanged snapshot). Cheap coverage gap. +4. ~~**In-memory duplicate instance id** — Plan §7 lists duplicate instance id deny; no explicit unit test calls `TryCreateActive` twice with the same instance id (second call should return `false` with unchanged snapshot). Cheap coverage gap.~~ **Done.** — `TryCreateActive_ShouldDenyDuplicateInstanceId`. -5. **Postgres outcome FK parity** — `PostgresContractOutcomeStore.TryAppend` will throw on FK violation (missing `contract_instance` row) while in-memory accepts any normalized row. Orchestrators (NEO-149) will create the instance first, so this is low risk today; document in README or catch `PostgresException` and return `false` if you want strict bool-only store semantics across backends. +5. ~~**Postgres outcome FK parity** — `PostgresContractOutcomeStore.TryAppend` will throw on FK violation (missing `contract_instance` row) while in-memory accepts any normalized row. Orchestrators (NEO-149) will create the instance first, so this is low risk today; document in README or catch `PostgresException` and return `false` if you want strict bool-only store semantics across backends.~~ **Done.** — FK violation caught; README documents orchestrator ordering. ## Nits -- Nit: `ContractInstanceIds.MakeInstanceKey` is defined but unused; plan also lists `MakeActivePlayerKey` which was not added. Either wire keys into store indexing or trim plan/reconciliation to match shipped API. +- ~~Nit: `ContractInstanceIds.MakeInstanceKey` is defined but unused; plan also lists `MakeActivePlayerKey` which was not added. Either wire keys into store indexing or trim plan/reconciliation to match shipped API.~~ **Done.** — `MakeInstanceKey` wired for per-row locks; plan trimmed; player-level lock for one-active create path. - Nit: `ContractInstanceReasonCodes` is defined for E7M4-04+ orchestrators but unused in this slice — correct deferral; no action until NEO-147. -- Nit: Bruno folder `bruno/neon-sprawl-server/contract-stores/` (health smoke) is useful but not listed in plan §Implementation reconciliation — optional one-line note for doc parity (same as NEO-145 Bruno nit). +- ~~Nit: Bruno folder `bruno/neon-sprawl-server/contract-stores/` (health smoke) is useful but not listed in plan §Implementation reconciliation — optional one-line note for doc parity (same as NEO-145 Bruno nit).~~ **Done.** — plan reconciliation updated. -- Nit: Plan open questions marked Postgres `TryClear*` as deferred; both Postgres stores implement clear helpers anyway — update plan risk row to `adopted` on next doc pass. +- ~~Nit: Plan open questions marked Postgres `TryClear*` as deferred; both Postgres stores implement clear helpers anyway — update plan risk row to `adopted` on next doc pass.~~ **Done.** ## Verification @@ -67,4 +67,4 @@ Author should run Postgres integration locally when `ConnectionStrings__NeonSpra dotnet test NeonSprawl.sln --filter "FullyQualifiedName~ContractInstancePersistence|FullyQualifiedName~ContractOutcomePersistence" ``` -Before merge: skim decomposition alignment suggestion — functional code is merge-ready without it. +~~Before merge: skim decomposition alignment suggestion — functional code is merge-ready without it.~~ **Done** — review follow-up committed. diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractInstancePersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractInstancePersistenceIntegrationTests.cs index 4fa4f9e..1f9a5bd 100644 --- a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractInstancePersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractInstancePersistenceIntegrationTests.cs @@ -53,6 +53,70 @@ public sealed class ContractInstancePersistenceIntegrationTests(PostgresIntegrat Assert.Equal(TemplateId, snapshot.TemplateId); } + [RequirePostgresFact] + public async Task TryCreateActive_ShouldAllowOnlyOneActive_WhenDistinctInstanceIdsRace() + { + // Arrange + await ResetContractInstanceTableAsync(); + const int concurrentCalls = 8; + + // Act + var results = await Task.WhenAll( + Enumerable.Range(0, concurrentCalls).Select(i => Task.Run(() => + { + using var scope = Factory.Services.CreateScope(); + var store = scope.ServiceProvider.GetRequiredService(); + var instanceId = $"prototype_contract_instance_pg_race_{i:D3}"; + return ( + Success: store.TryCreateActive( + PlayerId, + instanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _), + InstanceId: instanceId); + }))); + + // Assert + Assert.Equal(1, results.Count(static r => r.Success)); + Assert.Equal(concurrentCalls - 1, results.Count(static r => !r.Success)); + Assert.True( + Factory.Services.CreateScope().ServiceProvider + .GetRequiredService() + .TryGetActiveForPlayer(PlayerId, out var active)); + Assert.Equal(ContractInstanceStatus.Active, active.Status); + Assert.Contains(results.Where(static r => r.Success).Select(static r => r.InstanceId), id => id == active.ContractInstanceId); + } + + [RequirePostgresFact] + public async Task TryMarkComplete_ShouldBeIdempotent_OnPostgres() + { + // Arrange + await ResetContractInstanceTableAsync(); + using var scope = Factory.Services.CreateScope(); + var store = scope.ServiceProvider.GetRequiredService(); + Assert.True(store.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _)); + // Act + var first = store.TryMarkComplete(PlayerId, InstanceId, CompletedAt, out var firstSnapshot); + var second = store.TryMarkComplete( + PlayerId, + InstanceId, + CompletedAt.AddHours(1), + out var secondSnapshot); + // Assert + Assert.True(first); + Assert.False(second); + Assert.Equal(CompletedAt, secondSnapshot.CompletedAt); + Assert.Equal(firstSnapshot.ContractInstanceId, secondSnapshot.ContractInstanceId); + } + [RequirePostgresFact] public async Task CreateComplete_ShouldPersistAcrossNewFactory() { diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs index 3fd1bae..e85768d 100644 --- a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs @@ -71,6 +71,69 @@ public sealed class ContractOutcomePersistenceIntegrationTests(PostgresIntegrati Assert.Equal(15, readBack.GrantedSkillXp[0].Amount); } + [RequirePostgresFact] + public async Task TryAppend_ShouldDenyDuplicateRowIdAndIdempotencyKey() + { + // Arrange + await ResetContractTablesAsync(); + var idempotencyKey = ContractOutcomeIds.MakeIdempotencyKey(PlayerId, InstanceId); + var outcomeRow = new ContractOutcomeRow( + OutcomeRowId, + InstanceId, + PlayerId, + idempotencyKey, + [new RewardItemGrantApplied("scrap_metal_bulk", 5)], + [new RewardSkillXpGrantApplied("salvage", 15)], + [], + RecordedAt); + + using (var scope = Factory.Services.CreateScope()) + { + var instanceStore = scope.ServiceProvider.GetRequiredService(); + var outcomeStore = scope.ServiceProvider.GetRequiredService(); + Assert.True(instanceStore.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _)); + Assert.True(outcomeStore.TryAppend(outcomeRow)); + // Act + var duplicateId = outcomeStore.TryAppend(outcomeRow with { IdempotencyKey = "other-key" }); + var duplicateKey = outcomeStore.TryAppend(outcomeRow with { Id = "outcome-row-pg-002" }); + // Assert + Assert.False(duplicateId); + Assert.False(duplicateKey); + } + } + + [RequirePostgresFact] + public async Task TryAppend_ShouldReturnFalse_WhenContractInstanceMissing() + { + // Arrange + await ResetContractTablesAsync(); + var idempotencyKey = ContractOutcomeIds.MakeIdempotencyKey(PlayerId, InstanceId); + var outcomeRow = new ContractOutcomeRow( + OutcomeRowId, + InstanceId, + PlayerId, + idempotencyKey, + [new RewardItemGrantApplied("scrap_metal_bulk", 5)], + [], + [], + RecordedAt); + // Act + bool appended; + using (var scope = Factory.Services.CreateScope()) + { + var outcomeStore = scope.ServiceProvider.GetRequiredService(); + appended = outcomeStore.TryAppend(outcomeRow); + } + // Assert + Assert.False(appended); + } + private async Task ResetContractTablesAsync() { var cs = Environment.GetEnvironmentVariable("ConnectionStrings__NeonSprawl"); diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs index eaae754..8756c34 100644 --- a/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs @@ -53,6 +53,33 @@ public sealed class InMemoryContractInstanceStoreTests Assert.Null(snapshot.CompletedAt); } + [Fact] + public void TryCreateActive_ShouldDenyDuplicateInstanceId() + { + // Arrange + var store = CreateStore(); + Assert.True(store.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out var firstSnapshot)); + // Act + var duplicate = store.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt.AddHours(1), + out var secondSnapshot); + // Assert + Assert.False(duplicate); + Assert.Equal(firstSnapshot.ContractInstanceId, secondSnapshot.ContractInstanceId); + Assert.Equal(firstSnapshot.IssuedAt, secondSnapshot.IssuedAt); + Assert.Equal(ContractInstanceStatus.Active, secondSnapshot.Status); + } + [Fact] public void TryCreateActive_ShouldDenySecondActiveForSamePlayer() { diff --git a/server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs b/server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs index 8b90fb2..8b38e8b 100644 --- a/server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs +++ b/server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs @@ -48,6 +48,8 @@ public sealed class InMemoryContractInstanceStore(IOptions private readonly ConcurrentDictionary instanceLocks = new(StringComparer.Ordinal); + private static string PlayerLockKey(string player) => $"{player}\0active"; + private static HashSet CreateKnownPlayers(GamePositionOptions o) { var id = ContractInstanceIds.NormalizePlayerId(o.DevPlayerId); @@ -77,7 +79,13 @@ public sealed class InMemoryContractInstanceStore(IOptions return false; } - lock (instanceLocks.GetOrAdd(instanceId, _ => new object())) + var lockKey = ContractInstanceIds.MakeInstanceKey(player, instanceId); + if (lockKey.Length == 0) + { + return false; + } + + lock (instanceLocks.GetOrAdd(lockKey, _ => new object())) { if (!byInstanceId.TryGetValue(instanceId, out var row) || !string.Equals(row.PlayerId, player, StringComparison.Ordinal)) @@ -105,7 +113,7 @@ public sealed class InMemoryContractInstanceStore(IOptions return false; } - lock (instanceLocks.GetOrAdd(instanceId, _ => new object())) + lock (instanceLocks.GetOrAdd(PlayerLockKey(player), _ => new object())) { if (!byInstanceId.TryGetValue(instanceId, out var row) || row.Status != ContractInstanceStatus.Active || @@ -143,7 +151,7 @@ public sealed class InMemoryContractInstanceStore(IOptions return false; } - lock (instanceLocks.GetOrAdd(instanceId, _ => new object())) + lock (instanceLocks.GetOrAdd(PlayerLockKey(player), _ => new object())) { if (activeInstanceByPlayer.TryGetValue(player, out var existingActiveId)) { @@ -193,7 +201,13 @@ public sealed class InMemoryContractInstanceStore(IOptions return false; } - lock (instanceLocks.GetOrAdd(instanceId, _ => new object())) + var lockKey = ContractInstanceIds.MakeInstanceKey(player, instanceId); + if (lockKey.Length == 0) + { + return false; + } + + lock (instanceLocks.GetOrAdd(lockKey, _ => new object())) { if (!byInstanceId.TryGetValue(instanceId, out var row) || !string.Equals(row.PlayerId, player, StringComparison.Ordinal)) @@ -230,7 +244,13 @@ public sealed class InMemoryContractInstanceStore(IOptions return false; } - lock (instanceLocks.GetOrAdd(instanceId, _ => new object())) + var lockKey = ContractInstanceIds.MakeInstanceKey(player, instanceId); + if (lockKey.Length == 0) + { + return false; + } + + lock (instanceLocks.GetOrAdd(lockKey, _ => new object())) { if (!byInstanceId.TryGetValue(instanceId, out var row) || !string.Equals(row.PlayerId, player, StringComparison.Ordinal)) diff --git a/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs b/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs index 222c05c..4089fff 100644 --- a/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs +++ b/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs @@ -40,7 +40,14 @@ public sealed class PostgresContractOutcomeStore(Npgsql.NpgsqlDataSource dataSou cmd.Parameters.AddWithValue("skill_xp", SerializeGrants(normalized.GrantedSkillXp)); cmd.Parameters.AddWithValue("rep", SerializeGrants(normalized.GrantedReputation)); cmd.Parameters.AddWithValue("recorded", normalized.RecordedAt); - return cmd.ExecuteNonQuery() == 1; + try + { + return cmd.ExecuteNonQuery() == 1; + } + catch (Npgsql.PostgresException ex) when (ex.SqlState == Npgsql.PostgresErrorCodes.ForeignKeyViolation) + { + return false; + } } /// diff --git a/server/README.md b/server/README.md index c4e6282..1f79191 100644 --- a/server/README.md +++ b/server/README.md @@ -292,7 +292,7 @@ Append-only contract completion audit rows live in **`IContractOutcomeStore`**. **Store interface methods:** -- **`TryAppend`** — first row id or idempotency key returns `true`; duplicates return `false`. +- **`TryAppend`** — first row id or idempotency key returns `true`; duplicates return `false`. Missing **`contract_instance`** row (Postgres FK) also returns `false` — orchestrators (NEO-149) must create the instance before appending an outcome. - **`TryGetByIdempotencyKey`** — read one outcome for delivery replay / audit tests. - **`GetOutcomesForInstance`** — list outcomes for one contract instance id (ordered by recorded time). - **`TryClearForInstance`** — dev fixture only. From b455ff5ad54e88f32837320fd4b9370ee8a27de0 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 24 Jun 2026 20:41:25 -0400 Subject: [PATCH 05/18] NEO-146: apply C# collection and using style fixes Replace .ToArray() with collection spread, simplify single-statement await using blocks, use Lock for Postgres schema gates, and document IDE0063/IDE0305/IDE0330/CA1859 conventions in csharp-style.md. --- .cursor/rules/csharp-style.md | 56 ++++++++++++++++++- ...otbarLoadoutPersistenceIntegrationTests.cs | 25 ++++----- ...ractInstancePersistenceIntegrationTests.cs | 32 ++++------- ...tractOutcomePersistenceIntegrationTests.cs | 47 ++++++---------- ...tionStandingPersistenceIntegrationTests.cs | 37 +++++------- .../InMemoryReputationDeltaStoreTests.cs | 2 +- ...utationDeltaPersistenceIntegrationTests.cs | 28 ++++------ ...NodeInstancePersistenceIntegrationTests.cs | 30 ++++------ ...ressionGrantPersistenceIntegrationTests.cs | 24 +++----- ...yerInventoryPersistenceIntegrationTests.cs | 42 ++++++-------- .../Mastery/MasteryCatalogRegistryTests.cs | 4 +- .../Game/Mastery/PerkStateApiTests.cs | 7 +-- .../PerkStatePersistenceIntegrationTests.cs | 43 ++++++-------- .../PostgresPositionStateIntegrationTests.cs | 23 +++----- ...uestProgressPersistenceIntegrationTests.cs | 48 +++++++--------- .../Quests/QuestDefinitionsWorldApiTests.cs | 4 +- .../Game/Quests/QuestProgressApiTests.cs | 5 +- ...ressionGrantPersistenceIntegrationTests.cs | 35 +++++------- .../PostgresHotbarLoadoutBootstrap.cs | 4 +- .../Combat/AbilityDefinitionCatalogLoader.cs | 5 +- .../Game/Combat/AbilityDefinitionRegistry.cs | 2 +- .../ContractTemplateCatalogLoader.cs | 5 +- .../Contracts/ContractTemplateRegistry.cs | 2 +- .../Contracts/InMemoryContractOutcomeStore.cs | 13 ++--- .../PostgresContractInstanceBootstrap.cs | 4 +- .../PostgresContractOutcomeBootstrap.cs | 4 +- .../Contracts/PostgresContractOutcomeStore.cs | 12 ++-- .../Crafting/RecipeDefinitionCatalogLoader.cs | 5 +- .../Game/Crafting/RecipeDefinitionRegistry.cs | 2 +- .../EncounterDefinitionCatalogLoader.cs | 5 +- .../Encounters/EncounterDefinitionRegistry.cs | 2 +- .../InMemoryEncounterCompleteEventStore.cs | 2 +- .../RewardTableDefinitionCatalogLoader.cs | 5 +- .../RewardTableDefinitionRegistry.cs | 2 +- .../FactionDefinitionCatalogLoader.cs | 5 +- .../Factions/FactionDefinitionRegistry.cs | 2 +- .../Factions/InMemoryReputationDeltaStore.cs | 7 +-- .../PostgresPlayerFactionStandingBootstrap.cs | 4 +- .../PostgresReputationDeltaBootstrap.cs | 4 +- .../PostgresResourceNodeInstanceBootstrap.cs | 4 +- .../Gathering/ResourceNodeCatalogLoader.cs | 10 ++-- .../Gigs/PostgresGigProgressionBootstrap.cs | 4 +- .../Game/Items/ItemDefinitionCatalogLoader.cs | 5 +- .../Game/Items/ItemDefinitionRegistry.cs | 2 +- .../Items/PostgresPlayerInventoryBootstrap.cs | 4 +- .../Game/Mastery/MasteryCatalogLoader.cs | 5 +- .../Mastery/PostgresPerkStateBootstrap.cs | 4 +- .../Npc/NpcBehaviorDefinitionCatalogLoader.cs | 5 +- .../Game/Npc/NpcBehaviorDefinitionRegistry.cs | 2 +- .../PostgresPositionBootstrap.cs | 11 ++-- .../PostgresPlayerQuestProgressBootstrap.cs | 4 +- .../Quests/PrototypeE7M2QuestCatalogRules.cs | 10 ++-- .../Quests/PrototypeE7M3QuestFactionRules.cs | 15 ++--- .../Quests/QuestDefinitionCatalogLoader.cs | 5 +- .../Game/Quests/QuestDefinitionRegistry.cs | 2 +- .../Rewards/InMemoryRewardDeliveryStore.cs | 6 +- .../PostgresSkillProgressionBootstrap.cs | 4 +- .../Skills/SkillDefinitionCatalogLoader.cs | 5 +- .../Game/Skills/SkillDefinitionRegistry.cs | 2 +- 59 files changed, 329 insertions(+), 368 deletions(-) diff --git a/.cursor/rules/csharp-style.md b/.cursor/rules/csharp-style.md index 4f3e4b9..1201727 100644 --- a/.cursor/rules/csharp-style.md +++ b/.cursor/rules/csharp-style.md @@ -54,7 +54,61 @@ if (string.IsNullOrEmpty(key)) - **File-scoped namespaces** (`namespace X;`) for new single-namespace files when the SDK/version allows. - **Pattern matching / nullability:** prefer modern C# features where they simplify code; honor **nullable reference type** annotations when the project enables them. -## Members +## Collection expressions (IDE0305) + +- Prefer **collection spread** (`[.. source]`) over **`.ToArray()`** / **`.ToList()`** when materializing a sequence into a new array or list for return values, record `with` copies, or local snapshots. +- Give an **explicit target type** when the compiler cannot infer one (CS9176), e.g. `string[] ids = [.. query.Select(static x => x.Id)];`. +- In **assertions**, avoid passing a bare spread directly to APIs with span overloads (e.g. `Assert.Equal`) — assign to a typed local first: + +```csharp +// Prefer +return [.. query]; +GrantedItems = [.. row.GrantedItems], + +// When type inference fails +string[] expectedOrder = [.. registry.GetDefinitionsInIdOrder().Select(static d => d.Id)]; +string[] actualOrder = [.. body.Quests.Select(static row => row.QuestId)]; +Assert.Equal(expectedOrder, actualOrder); + +// Avoid (ambiguous Assert overloads; unnecessary allocation helper) +Assert.Equal(expectedOrder, [.. body.Quests.Select(static row => row.QuestId)]); +return query.ToArray(); +``` + +## `using` / `await using` (IDE0063) + +- When a **`using` or `await using` block contains exactly one statement**, prefer the declaration form without braces: `await using var conn = …;` then the single statement on following lines **only if** it is still logically one resource scope (typical for `NpgsqlConnection` + one command in integration-test reset helpers). +- **Keep braced `using (var …) { … }`** when the block has **multiple statements** (Arrange + Act + Assert inside one scope, seed loops, or any sequence that shares locals). +- **Do not flatten** multi-statement blocks into `using var` at method scope when inner scopes reuse names like `store`, `registry`, or `scope` — that causes **CS0136** name clashes. Use distinct names (`seedStore`, `actScope`, `readStore`) or keep the braced block. + +```csharp +// Prefer — single disposable, single follow-on statement +await using var conn = new NpgsqlConnection(cs); +await conn.OpenAsync(); + +// Prefer — multiple statements in one scope +using (var firstScope = Factory.Services.CreateScope()) +{ + var store = firstScope.ServiceProvider.GetRequiredService(); + Assert.True(store.TryAppend(row)); +} + +// Avoid — flattens to method scope and collides with a later `var store` +using var firstScope = Factory.Services.CreateScope(); +var store = firstScope.ServiceProvider.GetRequiredService(); +Assert.True(store.TryAppend(row)); +// … later … +var store = secondScope.ServiceProvider.GetRequiredService(); // CS0136 +``` + +## Postgres schema gates (IDE0330) + +- In `*Bootstrap.cs` types, use **`System.Threading.Lock`** (not `object`) for static schema-initialization gates: `private static readonly Lock SchemaGate = new();`. + +## Concrete return types (CA1859) + +- When a private helper always returns a materialized array, prefer the **concrete array type** as the return type (e.g. `RewardItemGrantApplied[]`) instead of `IReadOnlyList` if callers only need the array and analyzers suggest it. + - Prefer **expression-bodied** members only when they stay one clear idea. - **LINQ:** favor readability over micro-chains; break complex queries across lines. diff --git a/server/NeonSprawl.Server.Tests/Game/AbilityInput/HotbarLoadoutPersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/AbilityInput/HotbarLoadoutPersistenceIntegrationTests.cs index 1ea4c61..3d838bc 100644 --- a/server/NeonSprawl.Server.Tests/Game/AbilityInput/HotbarLoadoutPersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/AbilityInput/HotbarLoadoutPersistenceIntegrationTests.cs @@ -37,13 +37,12 @@ public sealed class HotbarLoadoutPersistenceIntegrationTests(PostgresIntegration HttpStatusCode postStatus; // Act - using (var firstClient = Factory.CreateClient()) - { - var post = await firstClient.PostAsJsonAsync( - "/game/players/dev-local-1/hotbar-loadout", - update); - postStatus = post.StatusCode; - } + using var firstClient = Factory.CreateClient(); + var post = await firstClient.PostAsJsonAsync( + "/game/players/dev-local-1/hotbar-loadout", + update); + postStatus = post.StatusCode; + // Assert await using var secondFactory = new PostgresWebApplicationFactory(); @@ -73,14 +72,10 @@ public sealed class HotbarLoadoutPersistenceIntegrationTests(PostgresIntegration var ddl = await File.ReadAllTextAsync(ddlPath); await using var conn = new NpgsqlConnection(cs); await conn.OpenAsync(); - await using (var apply = new NpgsqlCommand(ddl, conn)) - { - await apply.ExecuteNonQueryAsync(); - } + await using var apply = new NpgsqlCommand(ddl, conn); + await apply.ExecuteNonQueryAsync(); - await using (var truncate = new NpgsqlCommand("TRUNCATE player_hotbar_loadout;", conn)) - { - await truncate.ExecuteNonQueryAsync(); - } + await using var truncate = new NpgsqlCommand("TRUNCATE player_hotbar_loadout;", conn); + await truncate.ExecuteNonQueryAsync(); } } diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractInstancePersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractInstancePersistenceIntegrationTests.cs index 1f9a5bd..f828ba4 100644 --- a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractInstancePersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractInstancePersistenceIntegrationTests.cs @@ -138,14 +138,12 @@ public sealed class ContractInstancePersistenceIntegrationTests(PostgresIntegrat } ContractInstanceState readBack; - await using (var secondFactory = new PostgresWebApplicationFactory()) - { - using var secondScope = secondFactory.Services.CreateScope(); - var store = secondScope.ServiceProvider.GetRequiredService(); - readBack = store.TryGet(PlayerId, InstanceId, out var snapshot) - ? snapshot - : null!; - } + await using var secondFactory = new PostgresWebApplicationFactory(); + using var secondScope = secondFactory.Services.CreateScope(); + var readStore = secondScope.ServiceProvider.GetRequiredService(); + readBack = readStore.TryGet(PlayerId, InstanceId, out var snapshot) + ? snapshot + : null!; // Assert Assert.NotNull(readBack); @@ -176,21 +174,15 @@ public sealed class ContractInstancePersistenceIntegrationTests(PostgresIntegrat var instanceDdl = await File.ReadAllTextAsync(instanceDdlPath); await using var conn = new NpgsqlConnection(cs); await conn.OpenAsync(); - await using (var applyPosition = new NpgsqlCommand(positionDdl, conn)) - { - await applyPosition.ExecuteNonQueryAsync(); - } + await using var applyPosition = new NpgsqlCommand(positionDdl, conn); + await applyPosition.ExecuteNonQueryAsync(); - await using (var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn)) - { - await truncate.ExecuteNonQueryAsync(); - } + await using var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn); + await truncate.ExecuteNonQueryAsync(); PostgresPositionBootstrap.SeedDevPlayer(conn, options); - await using (var applyInstance = new NpgsqlCommand(instanceDdl, conn)) - { - await applyInstance.ExecuteNonQueryAsync(); - } + await using var applyInstance = new NpgsqlCommand(instanceDdl, conn); + await applyInstance.ExecuteNonQueryAsync(); } } diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs index e85768d..11a386a 100644 --- a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs @@ -54,14 +54,12 @@ public sealed class ContractOutcomePersistenceIntegrationTests(PostgresIntegrati } ContractOutcomeRow readBack; - await using (var secondFactory = new PostgresWebApplicationFactory()) - { - using var secondScope = secondFactory.Services.CreateScope(); - var outcomeStore = secondScope.ServiceProvider.GetRequiredService(); - readBack = outcomeStore.TryGetByIdempotencyKey(idempotencyKey, out var row) - ? row - : null!; - } + await using var secondFactory = new PostgresWebApplicationFactory(); + using var secondScope = secondFactory.Services.CreateScope(); + var readOutcomeStore = secondScope.ServiceProvider.GetRequiredService(); + readBack = readOutcomeStore.TryGetByIdempotencyKey(idempotencyKey, out var row) + ? row + : null!; // Assert Assert.NotNull(readBack); @@ -125,11 +123,10 @@ public sealed class ContractOutcomePersistenceIntegrationTests(PostgresIntegrati RecordedAt); // Act bool appended; - using (var scope = Factory.Services.CreateScope()) - { - var outcomeStore = scope.ServiceProvider.GetRequiredService(); - appended = outcomeStore.TryAppend(outcomeRow); - } + using var scope = Factory.Services.CreateScope(); + var outcomeStore = scope.ServiceProvider.GetRequiredService(); + appended = outcomeStore.TryAppend(outcomeRow); + // Assert Assert.False(appended); } @@ -158,26 +155,18 @@ public sealed class ContractOutcomePersistenceIntegrationTests(PostgresIntegrati var outcomeDdl = await File.ReadAllTextAsync(outcomeDdlPath); await using var conn = new NpgsqlConnection(cs); await conn.OpenAsync(); - await using (var applyPosition = new NpgsqlCommand(positionDdl, conn)) - { - await applyPosition.ExecuteNonQueryAsync(); - } + await using var applyPosition = new NpgsqlCommand(positionDdl, conn); + await applyPosition.ExecuteNonQueryAsync(); - await using (var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn)) - { - await truncate.ExecuteNonQueryAsync(); - } + await using var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn); + await truncate.ExecuteNonQueryAsync(); PostgresPositionBootstrap.SeedDevPlayer(conn, options); - await using (var applyInstance = new NpgsqlCommand(instanceDdl, conn)) - { - await applyInstance.ExecuteNonQueryAsync(); - } + await using var applyInstance = new NpgsqlCommand(instanceDdl, conn); + await applyInstance.ExecuteNonQueryAsync(); - await using (var applyOutcome = new NpgsqlCommand(outcomeDdl, conn)) - { - await applyOutcome.ExecuteNonQueryAsync(); - } + await using var applyOutcome = new NpgsqlCommand(outcomeDdl, conn); + await applyOutcome.ExecuteNonQueryAsync(); } } diff --git a/server/NeonSprawl.Server.Tests/Game/Factions/FactionStandingPersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Factions/FactionStandingPersistenceIntegrationTests.cs index b276cba..2b69e04 100644 --- a/server/NeonSprawl.Server.Tests/Game/Factions/FactionStandingPersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Factions/FactionStandingPersistenceIntegrationTests.cs @@ -31,12 +31,10 @@ public sealed class FactionStandingPersistenceIntegrationTests(PostgresIntegrati } FactionStandingReadOutcome readBack; - await using (var secondFactory = new PostgresWebApplicationFactory()) - { - using var secondScope = secondFactory.Services.CreateScope(); - var store = secondScope.ServiceProvider.GetRequiredService(); - readBack = store.TryGetStanding(PlayerId, GridFactionId); - } + await using var secondFactory = new PostgresWebApplicationFactory(); + using var secondScope = secondFactory.Services.CreateScope(); + var readStore = secondScope.ServiceProvider.GetRequiredService(); + readBack = readStore.TryGetStanding(PlayerId, GridFactionId); // Assert Assert.True(writeOutcome.Success); @@ -69,11 +67,10 @@ public sealed class FactionStandingPersistenceIntegrationTests(PostgresIntegrati // Act FactionStandingReadOutcome readBack; - using (var scope = Factory.Services.CreateScope()) - { - var store = scope.ServiceProvider.GetRequiredService(); - readBack = store.TryGetStanding(PlayerId, GridFactionId); - } + using var scope = Factory.Services.CreateScope(); + var store = scope.ServiceProvider.GetRequiredService(); + readBack = store.TryGetStanding(PlayerId, GridFactionId); + // Assert Assert.True(readBack.Success); @@ -102,21 +99,15 @@ public sealed class FactionStandingPersistenceIntegrationTests(PostgresIntegrati var standingDdl = await File.ReadAllTextAsync(standingDdlPath); await using var conn = new NpgsqlConnection(cs); await conn.OpenAsync(); - await using (var applyPosition = new NpgsqlCommand(positionDdl, conn)) - { - await applyPosition.ExecuteNonQueryAsync(); - } + await using var applyPosition = new NpgsqlCommand(positionDdl, conn); + await applyPosition.ExecuteNonQueryAsync(); - await using (var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn)) - { - await truncate.ExecuteNonQueryAsync(); - } + await using var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn); + await truncate.ExecuteNonQueryAsync(); PostgresPositionBootstrap.SeedDevPlayer(conn, options); - await using (var applyStanding = new NpgsqlCommand(standingDdl, conn)) - { - await applyStanding.ExecuteNonQueryAsync(); - } + await using var applyStanding = new NpgsqlCommand(standingDdl, conn); + await applyStanding.ExecuteNonQueryAsync(); } } diff --git a/server/NeonSprawl.Server.Tests/Game/Factions/InMemoryReputationDeltaStoreTests.cs b/server/NeonSprawl.Server.Tests/Game/Factions/InMemoryReputationDeltaStoreTests.cs index 3f2a3d9..ab1506d 100644 --- a/server/NeonSprawl.Server.Tests/Game/Factions/InMemoryReputationDeltaStoreTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Factions/InMemoryReputationDeltaStoreTests.cs @@ -85,7 +85,7 @@ public sealed class InMemoryReputationDeltaStoreTests // Act var rows = store.GetDeltasForPlayer(PlayerId); // Assert - Assert.Equal(["delta-a", "delta-m", "delta-z"], rows.Select(static r => r.Id).ToArray()); + Assert.Equal(["delta-a", "delta-m", "delta-z"], [.. rows.Select(static r => r.Id)]); } [Fact] diff --git a/server/NeonSprawl.Server.Tests/Game/Factions/ReputationDeltaPersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Factions/ReputationDeltaPersistenceIntegrationTests.cs index 50e512f..f69a434 100644 --- a/server/NeonSprawl.Server.Tests/Game/Factions/ReputationDeltaPersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Factions/ReputationDeltaPersistenceIntegrationTests.cs @@ -40,12 +40,10 @@ public sealed class ReputationDeltaPersistenceIntegrationTests(PostgresIntegrati } IReadOnlyList readBack; - await using (var secondFactory = new PostgresWebApplicationFactory()) - { - using var secondScope = secondFactory.Services.CreateScope(); - var store = secondScope.ServiceProvider.GetRequiredService(); - readBack = store.GetDeltasForPlayer(PlayerId); - } + await using var secondFactory = new PostgresWebApplicationFactory(); + using var secondScope = secondFactory.Services.CreateScope(); + var readStore = secondScope.ServiceProvider.GetRequiredService(); + readBack = readStore.GetDeltasForPlayer(PlayerId); // Assert var persisted = Assert.Single(readBack); @@ -77,21 +75,15 @@ public sealed class ReputationDeltaPersistenceIntegrationTests(PostgresIntegrati var auditDdl = await File.ReadAllTextAsync(auditDdlPath); await using var conn = new NpgsqlConnection(cs); await conn.OpenAsync(); - await using (var applyPosition = new NpgsqlCommand(positionDdl, conn)) - { - await applyPosition.ExecuteNonQueryAsync(); - } + await using var applyPosition = new NpgsqlCommand(positionDdl, conn); + await applyPosition.ExecuteNonQueryAsync(); - await using (var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn)) - { - await truncate.ExecuteNonQueryAsync(); - } + await using var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn); + await truncate.ExecuteNonQueryAsync(); PostgresPositionBootstrap.SeedDevPlayer(conn, options); - await using (var applyAudit = new NpgsqlCommand(auditDdl, conn)) - { - await applyAudit.ExecuteNonQueryAsync(); - } + await using var applyAudit = new NpgsqlCommand(auditDdl, conn); + await applyAudit.ExecuteNonQueryAsync(); } } diff --git a/server/NeonSprawl.Server.Tests/Game/Gathering/ResourceNodeInstancePersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Gathering/ResourceNodeInstancePersistenceIntegrationTests.cs index eaa7c38..29f5ec6 100644 --- a/server/NeonSprawl.Server.Tests/Game/Gathering/ResourceNodeInstancePersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Gathering/ResourceNodeInstancePersistenceIntegrationTests.cs @@ -48,11 +48,11 @@ public sealed class ResourceNodeInstancePersistenceIntegrationTests(PostgresInte await ResetInstanceTableAsync(); using (var seedScope = Factory.Services.CreateScope()) { - var registry = seedScope.ServiceProvider.GetRequiredService(); - var store = seedScope.ServiceProvider.GetRequiredService(); + var seedRegistry = seedScope.ServiceProvider.GetRequiredService(); + var seedStore = seedScope.ServiceProvider.GetRequiredService(); for (var i = 0; i < 10; i++) { - _ = ResourceNodeInstanceOperations.TryCommitSuccessfulGatherDecrement(AlphaId, registry, store); + _ = ResourceNodeInstanceOperations.TryCommitSuccessfulGatherDecrement(AlphaId, seedRegistry, seedStore); } } @@ -60,13 +60,11 @@ public sealed class ResourceNodeInstancePersistenceIntegrationTests(PostgresInte ResourceNodeInstanceMutationOutcome denyOutcome; ResourceNodeInstanceSnapshot persisted; await using var secondFactory = new PostgresWebApplicationFactory(); - using (var scope = secondFactory.Services.CreateScope()) - { - var registry = scope.ServiceProvider.GetRequiredService(); - var store = scope.ServiceProvider.GetRequiredService(); - denyOutcome = ResourceNodeInstanceOperations.TryCommitSuccessfulGatherDecrement(AlphaId, registry, store); - store.TryGetRemainingGathers(AlphaId, out persisted); - } + using var actScope = secondFactory.Services.CreateScope(); + var actRegistry = actScope.ServiceProvider.GetRequiredService(); + var actStore = actScope.ServiceProvider.GetRequiredService(); + denyOutcome = ResourceNodeInstanceOperations.TryCommitSuccessfulGatherDecrement(AlphaId, actRegistry, actStore); + actStore.TryGetRemainingGathers(AlphaId, out persisted); // Assert Assert.Equal(ResourceNodeInstanceMutationKind.Denied, denyOutcome.Kind); @@ -92,14 +90,10 @@ public sealed class ResourceNodeInstancePersistenceIntegrationTests(PostgresInte var ddl = await File.ReadAllTextAsync(ddlPath); await using var conn = new NpgsqlConnection(cs); await conn.OpenAsync(); - await using (var apply = new NpgsqlCommand(ddl, conn)) - { - await apply.ExecuteNonQueryAsync(); - } + await using var apply = new NpgsqlCommand(ddl, conn); + await apply.ExecuteNonQueryAsync(); - await using (var truncate = new NpgsqlCommand("TRUNCATE resource_node_instance;", conn)) - { - await truncate.ExecuteNonQueryAsync(); - } + await using var truncate = new NpgsqlCommand("TRUNCATE resource_node_instance;", conn); + await truncate.ExecuteNonQueryAsync(); } } diff --git a/server/NeonSprawl.Server.Tests/Game/Gigs/GigProgressionGrantPersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Gigs/GigProgressionGrantPersistenceIntegrationTests.cs index 0d2dd62..8b89444 100644 --- a/server/NeonSprawl.Server.Tests/Game/Gigs/GigProgressionGrantPersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Gigs/GigProgressionGrantPersistenceIntegrationTests.cs @@ -97,26 +97,18 @@ public sealed class GigProgressionGrantPersistenceIntegrationTests(PostgresInteg var gigDdl = await File.ReadAllTextAsync(gigDdlPath); await using var conn = new NpgsqlConnection(cs); await conn.OpenAsync(); - await using (var applyPosition = new NpgsqlCommand(positionDdl, conn)) - { - await applyPosition.ExecuteNonQueryAsync(); - } + await using var applyPosition = new NpgsqlCommand(positionDdl, conn); + await applyPosition.ExecuteNonQueryAsync(); - await using (var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn)) - { - await truncate.ExecuteNonQueryAsync(); - } + await using var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn); + await truncate.ExecuteNonQueryAsync(); PostgresPositionBootstrap.SeedDevPlayer(conn, options); - await using (var applyGig = new NpgsqlCommand(gigDdl, conn)) - { - await applyGig.ExecuteNonQueryAsync(); - } + await using var applyGig = new NpgsqlCommand(gigDdl, conn); + await applyGig.ExecuteNonQueryAsync(); - await using (var truncateGig = new NpgsqlCommand("TRUNCATE player_gig_progression;", conn)) - { - await truncateGig.ExecuteNonQueryAsync(); - } + await using var truncateGig = new NpgsqlCommand("TRUNCATE player_gig_progression;", conn); + await truncateGig.ExecuteNonQueryAsync(); } } diff --git a/server/NeonSprawl.Server.Tests/Game/Items/PlayerInventoryPersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Items/PlayerInventoryPersistenceIntegrationTests.cs index 37fcc29..17ec26b 100644 --- a/server/NeonSprawl.Server.Tests/Game/Items/PlayerInventoryPersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Items/PlayerInventoryPersistenceIntegrationTests.cs @@ -54,31 +54,31 @@ public sealed class PlayerInventoryPersistenceIntegrationTests(PostgresIntegrati await ResetInventoryTableAsync(); using (var seedScope = Factory.Services.CreateScope()) { - var registry = seedScope.ServiceProvider.GetRequiredService(); - var store = seedScope.ServiceProvider.GetRequiredService(); + var seedRegistry = seedScope.ServiceProvider.GetRequiredService(); + var seedStore = seedScope.ServiceProvider.GetRequiredService(); for (var i = 0; i < PlayerInventorySnapshot.BagSlotCount; i++) { _ = PlayerInventoryOperations.TryAddStack( "dev-local-1", "survey_drone_kit", quantity: 1, - registry, - store); + seedRegistry, + seedStore); } } // Act PlayerInventoryMutationOutcome denyOutcome; - using (var scope = Factory.Services.CreateScope()) + using (var actScope = Factory.Services.CreateScope()) { - var registry = scope.ServiceProvider.GetRequiredService(); - var store = scope.ServiceProvider.GetRequiredService(); + var actRegistry = actScope.ServiceProvider.GetRequiredService(); + var actStore = actScope.ServiceProvider.GetRequiredService(); denyOutcome = PlayerInventoryOperations.TryAddStack( "dev-local-1", "scrap_metal_bulk", quantity: 1, - registry, - store); + actRegistry, + actStore); } // Act — verify on fresh host @@ -120,27 +120,19 @@ public sealed class PlayerInventoryPersistenceIntegrationTests(PostgresIntegrati var inventoryDdl = await File.ReadAllTextAsync(inventoryDdlPath); await using var conn = new NpgsqlConnection(cs); await conn.OpenAsync(); - await using (var applyPosition = new NpgsqlCommand(positionDdl, conn)) - { - await applyPosition.ExecuteNonQueryAsync(); - } + await using var applyPosition = new NpgsqlCommand(positionDdl, conn); + await applyPosition.ExecuteNonQueryAsync(); - await using (var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn)) - { - await truncate.ExecuteNonQueryAsync(); - } + await using var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn); + await truncate.ExecuteNonQueryAsync(); PostgresPositionBootstrap.SeedDevPlayer(conn, options); - await using (var applyInventory = new NpgsqlCommand(inventoryDdl, conn)) - { - await applyInventory.ExecuteNonQueryAsync(); - } + await using var applyInventory = new NpgsqlCommand(inventoryDdl, conn); + await applyInventory.ExecuteNonQueryAsync(); - await using (var truncateInventory = new NpgsqlCommand("TRUNCATE player_inventory;", conn)) - { - await truncateInventory.ExecuteNonQueryAsync(); - } + await using var truncateInventory = new NpgsqlCommand("TRUNCATE player_inventory;", conn); + await truncateInventory.ExecuteNonQueryAsync(); } private static int OccupiedBagSlotCount(PlayerInventorySnapshot snapshot) diff --git a/server/NeonSprawl.Server.Tests/Game/Mastery/MasteryCatalogRegistryTests.cs b/server/NeonSprawl.Server.Tests/Game/Mastery/MasteryCatalogRegistryTests.cs index 44b47df..33330ce 100644 --- a/server/NeonSprawl.Server.Tests/Game/Mastery/MasteryCatalogRegistryTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Mastery/MasteryCatalogRegistryTests.cs @@ -89,7 +89,7 @@ public class MasteryCatalogRegistryTests // Act var tracks = registry.GetTracksInSkillIdOrder(); // Assert - Assert.Equal(["intrusion", "refine", "salvage"], tracks.Select(t => t.SkillId).ToArray()); + Assert.Equal(["intrusion", "refine", "salvage"], [.. tracks.Select(t => t.SkillId)]); } [Fact] @@ -111,7 +111,7 @@ public class MasteryCatalogRegistryTests // Act var ordered = registry.GetPerksInIdOrder(); // Assert - Assert.Equal(["a_perk", "m_perk", "z_perk"], ordered.Select(p => p.Id).ToArray()); + Assert.Equal(["a_perk", "m_perk", "z_perk"], [.. ordered.Select(p => p.Id)]); } [Fact] diff --git a/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs index d6372a4..67aeb35 100644 --- a/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs @@ -158,10 +158,9 @@ public sealed class PerkStateApiTests // Assert Assert.Equal(HttpStatusCode.OK, post.StatusCode); var json = await post.Content.ReadAsStringAsync(); - using (var doc = JsonDocument.Parse(json)) - { - Assert.False(doc.RootElement.TryGetProperty("reasonCode", out _)); - } + using var doc = JsonDocument.Parse(json); + Assert.False(doc.RootElement.TryGetProperty("reasonCode", out _)); + var envelope = JsonSerializer.Deserialize(json); Assert.NotNull(envelope); diff --git a/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStatePersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStatePersistenceIntegrationTests.cs index 776aff6..d06a1dd 100644 --- a/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStatePersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStatePersistenceIntegrationTests.cs @@ -20,23 +20,20 @@ public sealed class PerkStatePersistenceIntegrationTests(PostgresIntegrationHarn // Arrange await ResetPerkTablesAsync(); const string playerId = "dev-local-1"; - using (var scope = Factory.Services.CreateScope()) - { - var xpStore = scope.ServiceProvider.GetRequiredService(); - var engine = scope.ServiceProvider.GetRequiredService(); - Assert.True(xpStore.TryApplyXpDelta(playerId, "salvage", 450, out _, out _)); - Assert.True(engine.TrySelectBranch(playerId, "salvage", 1, "scrap_efficiency").Success); - _ = engine.ReevaluateAfterLevelUp(playerId, "salvage", newLevel: 4); - } + using var scope = Factory.Services.CreateScope(); + var xpStore = scope.ServiceProvider.GetRequiredService(); + var engine = scope.ServiceProvider.GetRequiredService(); + Assert.True(xpStore.TryApplyXpDelta(playerId, "salvage", 450, out _, out _)); + Assert.True(engine.TrySelectBranch(playerId, "salvage", 1, "scrap_efficiency").Success); + _ = engine.ReevaluateAfterLevelUp(playerId, "salvage", newLevel: 4); + // Act PerkStateSnapshot snapshot; - await using (var secondFactory = new PostgresWebApplicationFactory()) - { - using var scope = secondFactory.Services.CreateScope(); - var perkStore = scope.ServiceProvider.GetRequiredService(); - snapshot = perkStore.GetSnapshot(playerId); - } + await using var secondFactory = new PostgresWebApplicationFactory(); + using var secondScope = secondFactory.Services.CreateScope(); + var perkStore = secondScope.ServiceProvider.GetRequiredService(); + snapshot = perkStore.GetSnapshot(playerId); // Assert Assert.Equal("scrap_efficiency", snapshot.BranchPicksBySkillId["salvage"][1]); @@ -65,21 +62,15 @@ public sealed class PerkStatePersistenceIntegrationTests(PostgresIntegrationHarn var perkDdl = await File.ReadAllTextAsync(perkDdlPath); await using var conn = new NpgsqlConnection(cs); await conn.OpenAsync(); - await using (var applyPosition = new NpgsqlCommand(positionDdl, conn)) - { - await applyPosition.ExecuteNonQueryAsync(); - } + await using var applyPosition = new NpgsqlCommand(positionDdl, conn); + await applyPosition.ExecuteNonQueryAsync(); - await using (var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn)) - { - await truncate.ExecuteNonQueryAsync(); - } + await using var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn); + await truncate.ExecuteNonQueryAsync(); PostgresPositionBootstrap.SeedDevPlayer(conn, options); - await using (var applyPerk = new NpgsqlCommand(perkDdl, conn)) - { - await applyPerk.ExecuteNonQueryAsync(); - } + await using var applyPerk = new NpgsqlCommand(perkDdl, conn); + await applyPerk.ExecuteNonQueryAsync(); } } diff --git a/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresPositionStateIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresPositionStateIntegrationTests.cs index b0a6ef1..e47922c 100644 --- a/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresPositionStateIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresPositionStateIntegrationTests.cs @@ -62,12 +62,11 @@ public sealed class PostgresPositionStateIntegrationTests(PostgresIntegrationHar PositionStateResponse? persisted = null; // Act - using (var first = Factory.CreateClient()) - { - var post = await first.PostAsJsonAsync("/game/players/dev-local-1/move", cmd); - postStatus = post.StatusCode; - persisted = await post.Content.ReadFromJsonAsync(); - } + using var first = Factory.CreateClient(); + var post = await first.PostAsJsonAsync("/game/players/dev-local-1/move", cmd); + postStatus = post.StatusCode; + persisted = await post.Content.ReadFromJsonAsync(); + // Assert await using var secondFactory = new PostgresWebApplicationFactory(); @@ -122,15 +121,11 @@ public sealed class PostgresPositionStateIntegrationTests(PostgresIntegrationHar var ddl = await File.ReadAllTextAsync(ddlPath); await using var conn = new NpgsqlConnection(cs); await conn.OpenAsync(); - await using (var apply = new NpgsqlCommand(ddl, conn)) - { - await apply.ExecuteNonQueryAsync(); - } + await using var apply = new NpgsqlCommand(ddl, conn); + await apply.ExecuteNonQueryAsync(); - await using (var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn)) - { - await truncate.ExecuteNonQueryAsync(); - } + await using var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn); + await truncate.ExecuteNonQueryAsync(); PostgresPositionBootstrap.SeedDevPlayer(conn, options); } diff --git a/server/NeonSprawl.Server.Tests/Game/Quests/PlayerQuestProgressPersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Quests/PlayerQuestProgressPersistenceIntegrationTests.cs index 08b3063..60f43fa 100644 --- a/server/NeonSprawl.Server.Tests/Game/Quests/PlayerQuestProgressPersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Quests/PlayerQuestProgressPersistenceIntegrationTests.cs @@ -70,17 +70,15 @@ public sealed class PlayerQuestProgressPersistenceIntegrationTests(PostgresInteg // Act — read back through a fresh host QuestStepState readBack; - await using (var secondFactory = new PostgresWebApplicationFactory()) - { - using var secondScope = secondFactory.Services.CreateScope(); - var store = secondScope.ServiceProvider.GetRequiredService(); - readBack = store.TryGetProgress( - PlayerId, - PrototypeE7M1QuestCatalogRules.GatherIntroQuestId, - out var snapshot) - ? snapshot - : null!; - } + await using var secondFactory = new PostgresWebApplicationFactory(); + using var secondScope = secondFactory.Services.CreateScope(); + var readStore = secondScope.ServiceProvider.GetRequiredService(); + readBack = readStore.TryGetProgress( + PlayerId, + PrototypeE7M1QuestCatalogRules.GatherIntroQuestId, + out var snapshot) + ? snapshot + : null!; // Assert Assert.NotNull(readBack); @@ -108,12 +106,10 @@ public sealed class PlayerQuestProgressPersistenceIntegrationTests(PostgresInteg } var readBackExists = false; - await using (var secondFactory = new PostgresWebApplicationFactory()) - { - using var secondScope = secondFactory.Services.CreateScope(); - var store = secondScope.ServiceProvider.GetRequiredService(); - readBackExists = store.TryGetProgress(PlayerId, questId, out _); - } + await using var secondFactory = new PostgresWebApplicationFactory(); + using var secondScope = secondFactory.Services.CreateScope(); + var readStore = secondScope.ServiceProvider.GetRequiredService(); + readBackExists = readStore.TryGetProgress(PlayerId, questId, out _); // Assert Assert.False(readBackExists); @@ -141,21 +137,15 @@ public sealed class PlayerQuestProgressPersistenceIntegrationTests(PostgresInteg var questDdl = await File.ReadAllTextAsync(questDdlPath); await using var conn = new NpgsqlConnection(cs); await conn.OpenAsync(); - await using (var applyPosition = new NpgsqlCommand(positionDdl, conn)) - { - await applyPosition.ExecuteNonQueryAsync(); - } + await using var applyPosition = new NpgsqlCommand(positionDdl, conn); + await applyPosition.ExecuteNonQueryAsync(); - await using (var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn)) - { - await truncate.ExecuteNonQueryAsync(); - } + await using var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn); + await truncate.ExecuteNonQueryAsync(); PostgresPositionBootstrap.SeedDevPlayer(conn, options); - await using (var applyQuest = new NpgsqlCommand(questDdl, conn)) - { - await applyQuest.ExecuteNonQueryAsync(); - } + await using var applyQuest = new NpgsqlCommand(questDdl, conn); + await applyQuest.ExecuteNonQueryAsync(); } } diff --git a/server/NeonSprawl.Server.Tests/Game/Quests/QuestDefinitionsWorldApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Quests/QuestDefinitionsWorldApiTests.cs index a50c853..7ea7e59 100644 --- a/server/NeonSprawl.Server.Tests/Game/Quests/QuestDefinitionsWorldApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Quests/QuestDefinitionsWorldApiTests.cs @@ -24,8 +24,8 @@ public class QuestDefinitionsWorldApiTests Assert.NotNull(body.Quests); Assert.Equal(PrototypeE7M1QuestCatalogRules.ExpectedQuestIds.Count, body.Quests.Count); Assert.Equal( - PrototypeE7M1QuestCatalogRules.ExpectedQuestIds.Order(StringComparer.Ordinal).ToArray(), - body.Quests.Select(q => q.Id).ToArray()); + [.. PrototypeE7M1QuestCatalogRules.ExpectedQuestIds.Order(StringComparer.Ordinal)], + [.. body.Quests.Select(q => q.Id)]); var gatherIntro = body.Quests.Single(q => q.Id == "prototype_quest_gather_intro"); Assert.Equal("Intro: Salvage Run", gatherIntro.DisplayName); diff --git a/server/NeonSprawl.Server.Tests/Game/Quests/QuestProgressApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Quests/QuestProgressApiTests.cs index ddf974b..258bb65 100644 --- a/server/NeonSprawl.Server.Tests/Game/Quests/QuestProgressApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Quests/QuestProgressApiTests.cs @@ -63,7 +63,7 @@ public sealed class QuestProgressApiTests await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); var registry = factory.Services.GetRequiredService(); - var expectedQuestOrder = registry.GetDefinitionsInIdOrder().Select(static d => d.Id).ToArray(); + string[] expectedQuestOrder = [.. registry.GetDefinitionsInIdOrder().Select(static d => d.Id)]; // Act var response = await client.GetAsync($"/game/players/{PlayerId}/quest-progress"); @@ -75,7 +75,8 @@ public sealed class QuestProgressApiTests Assert.Equal(QuestProgressListResponse.CurrentSchemaVersion, body!.SchemaVersion); Assert.Equal(PlayerId, body.PlayerId); Assert.Equal(expectedQuestOrder.Length, body.Quests.Count); - Assert.Equal(expectedQuestOrder, body.Quests.Select(static row => row.QuestId).ToArray()); + string[] actualQuestOrder = [.. body.Quests.Select(static row => row.QuestId)]; + Assert.Equal(expectedQuestOrder, actualQuestOrder); foreach (var row in body.Quests) { Assert.Equal(QuestProgressApi.StatusNotStarted, row.Status); diff --git a/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantPersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantPersistenceIntegrationTests.cs index adf0d4a..dec654b 100644 --- a/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantPersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantPersistenceIntegrationTests.cs @@ -29,12 +29,11 @@ public sealed class SkillProgressionGrantPersistenceIntegrationTests(PostgresInt HttpResponseMessage postResponse; // Act — write grant through first host - using (var firstClient = Factory.CreateClient()) - { - postResponse = await firstClient.PostAsJsonAsync( - "/game/players/dev-local-1/skill-progression", - grant); - } + using var firstClient = Factory.CreateClient(); + postResponse = await firstClient.PostAsJsonAsync( + "/game/players/dev-local-1/skill-progression", + grant); + // Act — read back through a fresh host (forces DB round-trip / new DI scope) await using var secondFactory = new PostgresWebApplicationFactory(); @@ -78,26 +77,18 @@ public sealed class SkillProgressionGrantPersistenceIntegrationTests(PostgresInt var progressionDdl = await File.ReadAllTextAsync(progressionDdlPath); await using var conn = new NpgsqlConnection(cs); await conn.OpenAsync(); - await using (var applyPosition = new NpgsqlCommand(positionDdl, conn)) - { - await applyPosition.ExecuteNonQueryAsync(); - } + await using var applyPosition = new NpgsqlCommand(positionDdl, conn); + await applyPosition.ExecuteNonQueryAsync(); - await using (var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn)) - { - await truncate.ExecuteNonQueryAsync(); - } + await using var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn); + await truncate.ExecuteNonQueryAsync(); PostgresPositionBootstrap.SeedDevPlayer(conn, options); - await using (var applyProgression = new NpgsqlCommand(progressionDdl, conn)) - { - await applyProgression.ExecuteNonQueryAsync(); - } + await using var applyProgression = new NpgsqlCommand(progressionDdl, conn); + await applyProgression.ExecuteNonQueryAsync(); - await using (var truncateProgression = new NpgsqlCommand("TRUNCATE player_skill_progression;", conn)) - { - await truncateProgression.ExecuteNonQueryAsync(); - } + await using var truncateProgression = new NpgsqlCommand("TRUNCATE player_skill_progression;", conn); + await truncateProgression.ExecuteNonQueryAsync(); } } diff --git a/server/NeonSprawl.Server/Game/AbilityInput/PostgresHotbarLoadoutBootstrap.cs b/server/NeonSprawl.Server/Game/AbilityInput/PostgresHotbarLoadoutBootstrap.cs index 43ce203..d64e0c1 100644 --- a/server/NeonSprawl.Server/Game/AbilityInput/PostgresHotbarLoadoutBootstrap.cs +++ b/server/NeonSprawl.Server/Game/AbilityInput/PostgresHotbarLoadoutBootstrap.cs @@ -1,10 +1,12 @@ +using System.Threading; + namespace NeonSprawl.Server.Game.AbilityInput; /// Applies NEO-29 hotbar loadout table DDL once per process. public static class PostgresHotbarLoadoutBootstrap { private static readonly string DdlRelativePath = Path.Combine("db", "migrations", "V002__player_hotbar_loadout.sql"); - private static readonly object SchemaGate = new(); + private static readonly Lock SchemaGate = new(); private static int _schemaReady; public static void EnsureSchema(Npgsql.NpgsqlDataSource dataSource) diff --git a/server/NeonSprawl.Server/Game/Combat/AbilityDefinitionCatalogLoader.cs b/server/NeonSprawl.Server/Game/Combat/AbilityDefinitionCatalogLoader.cs index 255d8a2..781c5e9 100644 --- a/server/NeonSprawl.Server/Game/Combat/AbilityDefinitionCatalogLoader.cs +++ b/server/NeonSprawl.Server/Game/Combat/AbilityDefinitionCatalogLoader.cs @@ -26,9 +26,8 @@ public static class AbilityDefinitionCatalogLoader if (errors.Count > 0) ThrowIfAny(errors); - var jsonFiles = Directory.GetFiles(abilitiesDirectory, "*_abilities.json", SearchOption.TopDirectoryOnly) - .OrderBy(p => p, StringComparer.Ordinal) - .ToArray(); + string[] jsonFiles = [.. Directory.GetFiles(abilitiesDirectory, "*_abilities.json", SearchOption.TopDirectoryOnly) + .OrderBy(p => p, StringComparer.Ordinal)]; if (jsonFiles.Length == 0) errors.Add($"error: no *_abilities.json files under {abilitiesDirectory}"); diff --git a/server/NeonSprawl.Server/Game/Combat/AbilityDefinitionRegistry.cs b/server/NeonSprawl.Server/Game/Combat/AbilityDefinitionRegistry.cs index c47cbc9..2bafca8 100644 --- a/server/NeonSprawl.Server/Game/Combat/AbilityDefinitionRegistry.cs +++ b/server/NeonSprawl.Server/Game/Combat/AbilityDefinitionRegistry.cs @@ -43,7 +43,7 @@ public sealed class AbilityDefinitionRegistry(AbilityDefinitionCatalog catalog) private static IReadOnlyList BuildDefinitionsInIdOrder(AbilityDefinitionCatalog catalog) { - var ids = catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal).ToArray(); + string[] ids = [.. catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal)]; var list = new List(ids.Length); foreach (var id in ids) { diff --git a/server/NeonSprawl.Server/Game/Contracts/ContractTemplateCatalogLoader.cs b/server/NeonSprawl.Server/Game/Contracts/ContractTemplateCatalogLoader.cs index e04c072..d017909 100644 --- a/server/NeonSprawl.Server/Game/Contracts/ContractTemplateCatalogLoader.cs +++ b/server/NeonSprawl.Server/Game/Contracts/ContractTemplateCatalogLoader.cs @@ -62,9 +62,8 @@ public static class ContractTemplateCatalogLoader if (errors.Count > 0) ThrowIfAny(errors); - var jsonFiles = Directory.GetFiles(contractsDirectory, "*_contract_templates.json", SearchOption.TopDirectoryOnly) - .OrderBy(p => p, StringComparer.Ordinal) - .ToArray(); + string[] jsonFiles = [.. Directory.GetFiles(contractsDirectory, "*_contract_templates.json", SearchOption.TopDirectoryOnly) + .OrderBy(p => p, StringComparer.Ordinal)]; if (jsonFiles.Length == 0) errors.Add($"error: no *_contract_templates.json files under {contractsDirectory}"); diff --git a/server/NeonSprawl.Server/Game/Contracts/ContractTemplateRegistry.cs b/server/NeonSprawl.Server/Game/Contracts/ContractTemplateRegistry.cs index b9056f7..b64b7ac 100644 --- a/server/NeonSprawl.Server/Game/Contracts/ContractTemplateRegistry.cs +++ b/server/NeonSprawl.Server/Game/Contracts/ContractTemplateRegistry.cs @@ -27,7 +27,7 @@ public sealed class ContractTemplateRegistry(ContractTemplateCatalog catalog) : /// public IReadOnlyList GetDefinitionsInIdOrder() { - var ids = catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal).ToArray(); + string[] ids = [.. catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal)]; var list = new List(ids.Length); foreach (var id in ids) list.Add(catalog.ById[id]); diff --git a/server/NeonSprawl.Server/Game/Contracts/InMemoryContractOutcomeStore.cs b/server/NeonSprawl.Server/Game/Contracts/InMemoryContractOutcomeStore.cs index 842e2f1..0e502df 100644 --- a/server/NeonSprawl.Server/Game/Contracts/InMemoryContractOutcomeStore.cs +++ b/server/NeonSprawl.Server/Game/Contracts/InMemoryContractOutcomeStore.cs @@ -73,7 +73,7 @@ public sealed class InMemoryContractOutcomeStore : IContractOutcomeStore query = query.Take(limit.Value); } - return query.ToArray(); + return [..query]; } /// @@ -85,10 +85,9 @@ public sealed class InMemoryContractOutcomeStore : IContractOutcomeStore return false; } - var idsToRemove = rowsById.Values + string[] idsToRemove = [.. rowsById.Values .Where(row => string.Equals(row.ContractInstanceId, instanceId, StringComparison.Ordinal)) - .Select(static row => row.Id) - .ToArray(); + .Select(static row => row.Id)]; foreach (var id in idsToRemove) { @@ -124,9 +123,9 @@ public sealed class InMemoryContractOutcomeStore : IContractOutcomeStore PlayerId = playerId, ContractInstanceId = instanceId, IdempotencyKey = idempotencyKey, - GrantedItems = row.GrantedItems.ToArray(), - GrantedSkillXp = row.GrantedSkillXp.ToArray(), - GrantedReputation = row.GrantedReputation.ToArray(), + GrantedItems = [..row.GrantedItems], + GrantedSkillXp = [..row.GrantedSkillXp], + GrantedReputation = [..row.GrantedReputation], }; return true; } diff --git a/server/NeonSprawl.Server/Game/Contracts/PostgresContractInstanceBootstrap.cs b/server/NeonSprawl.Server/Game/Contracts/PostgresContractInstanceBootstrap.cs index c2e7de6..87fedd1 100644 --- a/server/NeonSprawl.Server/Game/Contracts/PostgresContractInstanceBootstrap.cs +++ b/server/NeonSprawl.Server/Game/Contracts/PostgresContractInstanceBootstrap.cs @@ -1,3 +1,5 @@ +using System.Threading; + namespace NeonSprawl.Server.Game.Contracts; /// Applies NEO-146 contract instance table DDL once per process. @@ -5,7 +7,7 @@ public static class PostgresContractInstanceBootstrap { private static readonly string DdlRelativePath = Path.Combine("db", "migrations", "V011__contract_instance.sql"); - private static readonly object SchemaGate = new(); + private static readonly Lock SchemaGate = new(); private static int _schemaReady; diff --git a/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeBootstrap.cs b/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeBootstrap.cs index a85e846..3e56365 100644 --- a/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeBootstrap.cs +++ b/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeBootstrap.cs @@ -1,3 +1,5 @@ +using System.Threading; + namespace NeonSprawl.Server.Game.Contracts; /// Applies NEO-146 contract outcome table DDL once per process. @@ -5,7 +7,7 @@ public static class PostgresContractOutcomeBootstrap { private static readonly string DdlRelativePath = Path.Combine("db", "migrations", "V012__contract_outcome.sql"); - private static readonly object SchemaGate = new(); + private static readonly Lock SchemaGate = new(); private static int _schemaReady; diff --git a/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs b/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs index 4089fff..fcd694d 100644 --- a/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs +++ b/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs @@ -162,19 +162,19 @@ public sealed class PostgresContractOutcomeStore(Npgsql.NpgsqlDataSource dataSou private static string SerializeGrants(IReadOnlyList grants) => JsonSerializer.Serialize(grants, JsonOptions); - private static IReadOnlyList DeserializeItems(string json) + private static RewardItemGrantApplied[] DeserializeItems(string json) { var parsed = JsonSerializer.Deserialize(json, JsonOptions); return parsed ?? []; } - private static IReadOnlyList DeserializeSkillXp(string json) + private static RewardSkillXpGrantApplied[] DeserializeSkillXp(string json) { var parsed = JsonSerializer.Deserialize(json, JsonOptions); return parsed ?? []; } - private static IReadOnlyList DeserializeReputation(string json) + private static RewardReputationGrantApplied[] DeserializeReputation(string json) { var parsed = JsonSerializer.Deserialize(json, JsonOptions); return parsed ?? []; @@ -201,9 +201,9 @@ public sealed class PostgresContractOutcomeStore(Npgsql.NpgsqlDataSource dataSou PlayerId = playerId, ContractInstanceId = instanceId, IdempotencyKey = idempotencyKey, - GrantedItems = row.GrantedItems.ToArray(), - GrantedSkillXp = row.GrantedSkillXp.ToArray(), - GrantedReputation = row.GrantedReputation.ToArray(), + GrantedItems = [..row.GrantedItems], + GrantedSkillXp = [..row.GrantedSkillXp], + GrantedReputation = [..row.GrantedReputation], }; return true; } diff --git a/server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionCatalogLoader.cs b/server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionCatalogLoader.cs index f43a8f4..0800c67 100644 --- a/server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionCatalogLoader.cs +++ b/server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionCatalogLoader.cs @@ -36,9 +36,8 @@ public static class RecipeDefinitionCatalogLoader if (errors.Count > 0) ThrowIfAny(errors); - var jsonFiles = Directory.GetFiles(recipesDirectory, "*_recipes.json", SearchOption.TopDirectoryOnly) - .OrderBy(p => p, StringComparer.Ordinal) - .ToArray(); + string[] jsonFiles = [.. Directory.GetFiles(recipesDirectory, "*_recipes.json", SearchOption.TopDirectoryOnly) + .OrderBy(p => p, StringComparer.Ordinal)]; if (jsonFiles.Length == 0) errors.Add($"error: no *_recipes.json files under {recipesDirectory}"); diff --git a/server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionRegistry.cs b/server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionRegistry.cs index 7b84018..206a400 100644 --- a/server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionRegistry.cs +++ b/server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionRegistry.cs @@ -24,7 +24,7 @@ public sealed class RecipeDefinitionRegistry(RecipeDefinitionCatalog catalog) : private static IReadOnlyList BuildDefinitionsInIdOrder(RecipeDefinitionCatalog catalog) { - var ids = catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal).ToArray(); + string[] ids = [.. catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal)]; var list = new List(ids.Length); foreach (var id in ids) { diff --git a/server/NeonSprawl.Server/Game/Encounters/EncounterDefinitionCatalogLoader.cs b/server/NeonSprawl.Server/Game/Encounters/EncounterDefinitionCatalogLoader.cs index 4acec77..32137f4 100644 --- a/server/NeonSprawl.Server/Game/Encounters/EncounterDefinitionCatalogLoader.cs +++ b/server/NeonSprawl.Server/Game/Encounters/EncounterDefinitionCatalogLoader.cs @@ -30,9 +30,8 @@ public static class EncounterDefinitionCatalogLoader if (errors.Count > 0) ThrowIfAny(errors); - var jsonFiles = Directory.GetFiles(encountersDirectory, "*_encounters.json", SearchOption.TopDirectoryOnly) - .OrderBy(p => p, StringComparer.Ordinal) - .ToArray(); + string[] jsonFiles = [.. Directory.GetFiles(encountersDirectory, "*_encounters.json", SearchOption.TopDirectoryOnly) + .OrderBy(p => p, StringComparer.Ordinal)]; if (jsonFiles.Length == 0) errors.Add($"error: no *_encounters.json files under {encountersDirectory}"); diff --git a/server/NeonSprawl.Server/Game/Encounters/EncounterDefinitionRegistry.cs b/server/NeonSprawl.Server/Game/Encounters/EncounterDefinitionRegistry.cs index ba911a2..5e04fb3 100644 --- a/server/NeonSprawl.Server/Game/Encounters/EncounterDefinitionRegistry.cs +++ b/server/NeonSprawl.Server/Game/Encounters/EncounterDefinitionRegistry.cs @@ -43,7 +43,7 @@ public sealed class EncounterDefinitionRegistry(EncounterDefinitionCatalog catal private static IReadOnlyList BuildDefinitionsInIdOrder(EncounterDefinitionCatalog catalog) { - var ids = catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal).ToArray(); + string[] ids = [.. catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal)]; var list = new List(ids.Length); foreach (var id in ids) { diff --git a/server/NeonSprawl.Server/Game/Encounters/InMemoryEncounterCompleteEventStore.cs b/server/NeonSprawl.Server/Game/Encounters/InMemoryEncounterCompleteEventStore.cs index 539b549..0f036d8 100644 --- a/server/NeonSprawl.Server/Game/Encounters/InMemoryEncounterCompleteEventStore.cs +++ b/server/NeonSprawl.Server/Game/Encounters/InMemoryEncounterCompleteEventStore.cs @@ -27,7 +27,7 @@ public sealed class InMemoryEncounterCompleteEventStore : IEncounterCompleteEven eventsByKey[key] = completeEvent with { - GrantedItems = completeEvent.GrantedItems.ToArray(), + GrantedItems = [..completeEvent.GrantedItems], }; return true; } diff --git a/server/NeonSprawl.Server/Game/Encounters/RewardTableDefinitionCatalogLoader.cs b/server/NeonSprawl.Server/Game/Encounters/RewardTableDefinitionCatalogLoader.cs index b510ede..9a71202 100644 --- a/server/NeonSprawl.Server/Game/Encounters/RewardTableDefinitionCatalogLoader.cs +++ b/server/NeonSprawl.Server/Game/Encounters/RewardTableDefinitionCatalogLoader.cs @@ -35,9 +35,8 @@ public static class RewardTableDefinitionCatalogLoader if (errors.Count > 0) ThrowIfAny(errors); - var jsonFiles = Directory.GetFiles(rewardTablesDirectory, "*_reward_tables.json", SearchOption.TopDirectoryOnly) - .OrderBy(p => p, StringComparer.Ordinal) - .ToArray(); + string[] jsonFiles = [.. Directory.GetFiles(rewardTablesDirectory, "*_reward_tables.json", SearchOption.TopDirectoryOnly) + .OrderBy(p => p, StringComparer.Ordinal)]; if (jsonFiles.Length == 0) errors.Add($"error: no *_reward_tables.json files under {rewardTablesDirectory}"); diff --git a/server/NeonSprawl.Server/Game/Encounters/RewardTableDefinitionRegistry.cs b/server/NeonSprawl.Server/Game/Encounters/RewardTableDefinitionRegistry.cs index b5d61dc..a7da70b 100644 --- a/server/NeonSprawl.Server/Game/Encounters/RewardTableDefinitionRegistry.cs +++ b/server/NeonSprawl.Server/Game/Encounters/RewardTableDefinitionRegistry.cs @@ -43,7 +43,7 @@ public sealed class RewardTableDefinitionRegistry(RewardTableDefinitionCatalog c private static IReadOnlyList BuildDefinitionsInIdOrder(RewardTableDefinitionCatalog catalog) { - var ids = catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal).ToArray(); + string[] ids = [.. catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal)]; var list = new List(ids.Length); foreach (var id in ids) { diff --git a/server/NeonSprawl.Server/Game/Factions/FactionDefinitionCatalogLoader.cs b/server/NeonSprawl.Server/Game/Factions/FactionDefinitionCatalogLoader.cs index 68fdbc7..6497c6e 100644 --- a/server/NeonSprawl.Server/Game/Factions/FactionDefinitionCatalogLoader.cs +++ b/server/NeonSprawl.Server/Game/Factions/FactionDefinitionCatalogLoader.cs @@ -26,9 +26,8 @@ public static class FactionDefinitionCatalogLoader if (errors.Count > 0) ThrowIfAny(errors); - var jsonFiles = Directory.GetFiles(factionsDirectory, "*_factions.json", SearchOption.TopDirectoryOnly) - .OrderBy(p => p, StringComparer.Ordinal) - .ToArray(); + string[] jsonFiles = [.. Directory.GetFiles(factionsDirectory, "*_factions.json", SearchOption.TopDirectoryOnly) + .OrderBy(p => p, StringComparer.Ordinal)]; if (jsonFiles.Length == 0) errors.Add($"error: no *_factions.json files under {factionsDirectory}"); diff --git a/server/NeonSprawl.Server/Game/Factions/FactionDefinitionRegistry.cs b/server/NeonSprawl.Server/Game/Factions/FactionDefinitionRegistry.cs index 277adcf..63a59e4 100644 --- a/server/NeonSprawl.Server/Game/Factions/FactionDefinitionRegistry.cs +++ b/server/NeonSprawl.Server/Game/Factions/FactionDefinitionRegistry.cs @@ -27,7 +27,7 @@ public sealed class FactionDefinitionRegistry(FactionDefinitionCatalog catalog) /// public IReadOnlyList GetDefinitionsInIdOrder() { - var ids = catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal).ToArray(); + string[] ids = [.. catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal)]; var list = new List(ids.Length); foreach (var id in ids) { diff --git a/server/NeonSprawl.Server/Game/Factions/InMemoryReputationDeltaStore.cs b/server/NeonSprawl.Server/Game/Factions/InMemoryReputationDeltaStore.cs index bc226c1..a33f2e4 100644 --- a/server/NeonSprawl.Server/Game/Factions/InMemoryReputationDeltaStore.cs +++ b/server/NeonSprawl.Server/Game/Factions/InMemoryReputationDeltaStore.cs @@ -58,7 +58,7 @@ public sealed class InMemoryReputationDeltaStore : IReputationDeltaStore query = query.Take(limit.Value); } - return query.ToArray(); + return [..query]; } /// @@ -71,13 +71,12 @@ public sealed class InMemoryReputationDeltaStore : IReputationDeltaStore return false; } - var idsToRemove = rowsById.Values + string[] idsToRemove = [.. rowsById.Values .Where(row => string.Equals(row.PlayerId, player, StringComparison.Ordinal) && string.Equals(row.SourceKind, ReputationDeltaSourceKinds.QuestCompletion, StringComparison.Ordinal) && string.Equals(row.SourceId, sourceId, StringComparison.Ordinal)) - .Select(static row => row.Id) - .ToArray(); + .Select(static row => row.Id)]; foreach (var id in idsToRemove) { diff --git a/server/NeonSprawl.Server/Game/Factions/PostgresPlayerFactionStandingBootstrap.cs b/server/NeonSprawl.Server/Game/Factions/PostgresPlayerFactionStandingBootstrap.cs index 72414ce..cddf10c 100644 --- a/server/NeonSprawl.Server/Game/Factions/PostgresPlayerFactionStandingBootstrap.cs +++ b/server/NeonSprawl.Server/Game/Factions/PostgresPlayerFactionStandingBootstrap.cs @@ -1,3 +1,5 @@ +using System.Threading; + namespace NeonSprawl.Server.Game.Factions; /// Applies NEO-135 faction standing table DDL once per process. @@ -5,7 +7,7 @@ public static class PostgresPlayerFactionStandingBootstrap { private static readonly string DdlRelativePath = Path.Combine("db", "migrations", "V009__player_faction_standing.sql"); - private static readonly object SchemaGate = new(); + private static readonly Lock SchemaGate = new(); private static int _schemaReady; diff --git a/server/NeonSprawl.Server/Game/Factions/PostgresReputationDeltaBootstrap.cs b/server/NeonSprawl.Server/Game/Factions/PostgresReputationDeltaBootstrap.cs index 30439e7..92f4b57 100644 --- a/server/NeonSprawl.Server/Game/Factions/PostgresReputationDeltaBootstrap.cs +++ b/server/NeonSprawl.Server/Game/Factions/PostgresReputationDeltaBootstrap.cs @@ -1,3 +1,5 @@ +using System.Threading; + namespace NeonSprawl.Server.Game.Factions; /// Applies NEO-135 reputation delta audit table DDL once per process. @@ -5,7 +7,7 @@ public static class PostgresReputationDeltaBootstrap { private static readonly string DdlRelativePath = Path.Combine("db", "migrations", "V010__reputation_delta_audit.sql"); - private static readonly object SchemaGate = new(); + private static readonly Lock SchemaGate = new(); private static int _schemaReady; diff --git a/server/NeonSprawl.Server/Game/Gathering/PostgresResourceNodeInstanceBootstrap.cs b/server/NeonSprawl.Server/Game/Gathering/PostgresResourceNodeInstanceBootstrap.cs index 8ba1705..f1fbca1 100644 --- a/server/NeonSprawl.Server/Game/Gathering/PostgresResourceNodeInstanceBootstrap.cs +++ b/server/NeonSprawl.Server/Game/Gathering/PostgresResourceNodeInstanceBootstrap.cs @@ -1,3 +1,5 @@ +using System.Threading; + namespace NeonSprawl.Server.Game.Gathering; /// Applies NEO-61 resource-node instance table DDL once per process. @@ -5,7 +7,7 @@ public static class PostgresResourceNodeInstanceBootstrap { private static readonly string DdlRelativePath = Path.Combine("db", "migrations", "V006__resource_node_instance.sql"); - private static readonly object SchemaGate = new(); + private static readonly Lock SchemaGate = new(); private static int _schemaReady; diff --git a/server/NeonSprawl.Server/Game/Gathering/ResourceNodeCatalogLoader.cs b/server/NeonSprawl.Server/Game/Gathering/ResourceNodeCatalogLoader.cs index 1563904..7945861 100644 --- a/server/NeonSprawl.Server/Game/Gathering/ResourceNodeCatalogLoader.cs +++ b/server/NeonSprawl.Server/Game/Gathering/ResourceNodeCatalogLoader.cs @@ -35,15 +35,13 @@ public static class ResourceNodeCatalogLoader if (errors.Count > 0) ThrowIfAny(errors); - var nodeJsonFiles = Directory.GetFiles(resourceNodesDirectory, "*_resource_nodes.json", SearchOption.TopDirectoryOnly) - .OrderBy(p => p, StringComparer.Ordinal) - .ToArray(); + string[] nodeJsonFiles = [.. Directory.GetFiles(resourceNodesDirectory, "*_resource_nodes.json", SearchOption.TopDirectoryOnly) + .OrderBy(p => p, StringComparer.Ordinal)]; if (nodeJsonFiles.Length == 0) errors.Add($"error: no *_resource_nodes.json files under {resourceNodesDirectory}"); - var yieldJsonFiles = Directory.GetFiles(resourceNodesDirectory, "*_resource_yields.json", SearchOption.TopDirectoryOnly) - .OrderBy(p => p, StringComparer.Ordinal) - .ToArray(); + string[] yieldJsonFiles = [.. Directory.GetFiles(resourceNodesDirectory, "*_resource_yields.json", SearchOption.TopDirectoryOnly) + .OrderBy(p => p, StringComparer.Ordinal)]; if (yieldJsonFiles.Length == 0) errors.Add($"error: no *_resource_yields.json files under {resourceNodesDirectory}"); diff --git a/server/NeonSprawl.Server/Game/Gigs/PostgresGigProgressionBootstrap.cs b/server/NeonSprawl.Server/Game/Gigs/PostgresGigProgressionBootstrap.cs index 905f6f4..5341f37 100644 --- a/server/NeonSprawl.Server/Game/Gigs/PostgresGigProgressionBootstrap.cs +++ b/server/NeonSprawl.Server/Game/Gigs/PostgresGigProgressionBootstrap.cs @@ -1,3 +1,5 @@ +using System.Threading; + namespace NeonSprawl.Server.Game.Gigs; /// Applies NEO-44 gig progression table DDL once per process. @@ -5,7 +7,7 @@ public static class PostgresGigProgressionBootstrap { private static readonly string DdlRelativePath = Path.Combine("db", "migrations", "V007__player_gig_progression.sql"); - private static readonly object SchemaGate = new(); + private static readonly Lock SchemaGate = new(); private static int _schemaReady; diff --git a/server/NeonSprawl.Server/Game/Items/ItemDefinitionCatalogLoader.cs b/server/NeonSprawl.Server/Game/Items/ItemDefinitionCatalogLoader.cs index 2edce09..78c85d8 100644 --- a/server/NeonSprawl.Server/Game/Items/ItemDefinitionCatalogLoader.cs +++ b/server/NeonSprawl.Server/Game/Items/ItemDefinitionCatalogLoader.cs @@ -26,9 +26,8 @@ public static class ItemDefinitionCatalogLoader if (errors.Count > 0) ThrowIfAny(errors); - var jsonFiles = Directory.GetFiles(itemsDirectory, "*_items.json", SearchOption.TopDirectoryOnly) - .OrderBy(p => p, StringComparer.Ordinal) - .ToArray(); + string[] jsonFiles = [.. Directory.GetFiles(itemsDirectory, "*_items.json", SearchOption.TopDirectoryOnly) + .OrderBy(p => p, StringComparer.Ordinal)]; if (jsonFiles.Length == 0) errors.Add($"error: no *_items.json files under {itemsDirectory}"); diff --git a/server/NeonSprawl.Server/Game/Items/ItemDefinitionRegistry.cs b/server/NeonSprawl.Server/Game/Items/ItemDefinitionRegistry.cs index c6af36a..9b236f3 100644 --- a/server/NeonSprawl.Server/Game/Items/ItemDefinitionRegistry.cs +++ b/server/NeonSprawl.Server/Game/Items/ItemDefinitionRegistry.cs @@ -27,7 +27,7 @@ public sealed class ItemDefinitionRegistry(ItemDefinitionCatalog catalog) : IIte /// public IReadOnlyList GetDefinitionsInIdOrder() { - var ids = catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal).ToArray(); + string[] ids = [.. catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal)]; var list = new List(ids.Length); foreach (var id in ids) { diff --git a/server/NeonSprawl.Server/Game/Items/PostgresPlayerInventoryBootstrap.cs b/server/NeonSprawl.Server/Game/Items/PostgresPlayerInventoryBootstrap.cs index 9286068..1ffa86c 100644 --- a/server/NeonSprawl.Server/Game/Items/PostgresPlayerInventoryBootstrap.cs +++ b/server/NeonSprawl.Server/Game/Items/PostgresPlayerInventoryBootstrap.cs @@ -1,3 +1,5 @@ +using System.Threading; + namespace NeonSprawl.Server.Game.Items; /// Applies NEO-54 inventory table DDL once per process. @@ -5,7 +7,7 @@ public static class PostgresPlayerInventoryBootstrap { private static readonly string DdlRelativePath = Path.Combine("db", "migrations", "V005__player_inventory.sql"); - private static readonly object SchemaGate = new(); + private static readonly Lock SchemaGate = new(); private static int _schemaReady; diff --git a/server/NeonSprawl.Server/Game/Mastery/MasteryCatalogLoader.cs b/server/NeonSprawl.Server/Game/Mastery/MasteryCatalogLoader.cs index 1afdf7d..63f53d8 100644 --- a/server/NeonSprawl.Server/Game/Mastery/MasteryCatalogLoader.cs +++ b/server/NeonSprawl.Server/Game/Mastery/MasteryCatalogLoader.cs @@ -29,9 +29,8 @@ public static class MasteryCatalogLoader if (errors.Count > 0) ThrowIfAny(errors); - var jsonFiles = Directory.GetFiles(masteryDirectory, "*_mastery.json", SearchOption.TopDirectoryOnly) - .OrderBy(p => p, StringComparer.Ordinal) - .ToArray(); + string[] jsonFiles = [.. Directory.GetFiles(masteryDirectory, "*_mastery.json", SearchOption.TopDirectoryOnly) + .OrderBy(p => p, StringComparer.Ordinal)]; if (jsonFiles.Length == 0) errors.Add($"error: no *_mastery.json files under {masteryDirectory}"); diff --git a/server/NeonSprawl.Server/Game/Mastery/PostgresPerkStateBootstrap.cs b/server/NeonSprawl.Server/Game/Mastery/PostgresPerkStateBootstrap.cs index 1d8ebfd..aea17f2 100644 --- a/server/NeonSprawl.Server/Game/Mastery/PostgresPerkStateBootstrap.cs +++ b/server/NeonSprawl.Server/Game/Mastery/PostgresPerkStateBootstrap.cs @@ -1,3 +1,5 @@ +using System.Threading; + namespace NeonSprawl.Server.Game.Mastery; /// Applies NEO-47 perk state table DDL once per process. @@ -5,7 +7,7 @@ public static class PostgresPerkStateBootstrap { private static readonly string DdlRelativePath = Path.Combine("db", "migrations", "V004__player_perk_state.sql"); - private static readonly object SchemaGate = new(); + private static readonly Lock SchemaGate = new(); private static int _schemaReady; diff --git a/server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefinitionCatalogLoader.cs b/server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefinitionCatalogLoader.cs index 6c16516..4838e98 100644 --- a/server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefinitionCatalogLoader.cs +++ b/server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefinitionCatalogLoader.cs @@ -26,9 +26,8 @@ public static class NpcBehaviorDefinitionCatalogLoader if (errors.Count > 0) ThrowIfAny(errors); - var jsonFiles = Directory.GetFiles(npcBehaviorsDirectory, "*_npc_behaviors.json", SearchOption.TopDirectoryOnly) - .OrderBy(p => p, StringComparer.Ordinal) - .ToArray(); + string[] jsonFiles = [.. Directory.GetFiles(npcBehaviorsDirectory, "*_npc_behaviors.json", SearchOption.TopDirectoryOnly) + .OrderBy(p => p, StringComparer.Ordinal)]; if (jsonFiles.Length == 0) errors.Add($"error: no *_npc_behaviors.json files under {npcBehaviorsDirectory}"); diff --git a/server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefinitionRegistry.cs b/server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefinitionRegistry.cs index 2ae0b96..9eb6581 100644 --- a/server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefinitionRegistry.cs +++ b/server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefinitionRegistry.cs @@ -43,7 +43,7 @@ public sealed class NpcBehaviorDefinitionRegistry(NpcBehaviorDefinitionCatalog c private static IReadOnlyList BuildDefinitionsInIdOrder(NpcBehaviorDefinitionCatalog catalog) { - var ids = catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal).ToArray(); + string[] ids = [.. catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal)]; var list = new List(ids.Length); foreach (var id in ids) { diff --git a/server/NeonSprawl.Server/Game/PositionState/PostgresPositionBootstrap.cs b/server/NeonSprawl.Server/Game/PositionState/PostgresPositionBootstrap.cs index 090c57a..47cbf47 100644 --- a/server/NeonSprawl.Server/Game/PositionState/PostgresPositionBootstrap.cs +++ b/server/NeonSprawl.Server/Game/PositionState/PostgresPositionBootstrap.cs @@ -1,3 +1,5 @@ +using System.Threading; + namespace NeonSprawl.Server.Game.PositionState; /// @@ -8,7 +10,7 @@ public static class PostgresPositionBootstrap { private static readonly string DdlRelativePath = Path.Combine("db", "migrations", "V001__player_position.sql"); - private static readonly object SchemaGate = new(); + private static readonly Lock SchemaGate = new(); private static int _schemaReady; /// @@ -36,10 +38,9 @@ public static class PostgresPositionBootstrap var ddl = File.ReadAllText(ddlPath); using var conn = dataSource.OpenConnection(); - using (var cmd = new Npgsql.NpgsqlCommand(ddl, conn)) - { - cmd.ExecuteNonQuery(); - } + using var cmd = new Npgsql.NpgsqlCommand(ddl, conn); + cmd.ExecuteNonQuery(); + System.Threading.Volatile.Write(ref _schemaReady, 1); } diff --git a/server/NeonSprawl.Server/Game/Quests/PostgresPlayerQuestProgressBootstrap.cs b/server/NeonSprawl.Server/Game/Quests/PostgresPlayerQuestProgressBootstrap.cs index 4d528db..db1cec5 100644 --- a/server/NeonSprawl.Server/Game/Quests/PostgresPlayerQuestProgressBootstrap.cs +++ b/server/NeonSprawl.Server/Game/Quests/PostgresPlayerQuestProgressBootstrap.cs @@ -1,3 +1,5 @@ +using System.Threading; + namespace NeonSprawl.Server.Game.Quests; /// Applies NEO-116 quest progress table DDL once per process. @@ -5,7 +7,7 @@ public static class PostgresPlayerQuestProgressBootstrap { private static readonly string DdlRelativePath = Path.Combine("db", "migrations", "V008__player_quest_progress.sql"); - private static readonly object SchemaGate = new(); + private static readonly Lock SchemaGate = new(); private static int _schemaReady; diff --git a/server/NeonSprawl.Server/Game/Quests/PrototypeE7M2QuestCatalogRules.cs b/server/NeonSprawl.Server/Game/Quests/PrototypeE7M2QuestCatalogRules.cs index 321638f..d98e531 100644 --- a/server/NeonSprawl.Server/Game/Quests/PrototypeE7M2QuestCatalogRules.cs +++ b/server/NeonSprawl.Server/Game/Quests/PrototypeE7M2QuestCatalogRules.cs @@ -126,14 +126,12 @@ public static class PrototypeE7M2QuestCatalogRules private static NormalizedBundle NormalizeBundle(QuestRewardBundleRow bundle) => new( - bundle.ItemGrants + [.. bundle.ItemGrants .OrderBy(g => g.ItemId, StringComparer.Ordinal) - .ThenBy(g => g.Quantity) - .ToArray(), - bundle.SkillXpGrants + .ThenBy(g => g.Quantity)], + [.. bundle.SkillXpGrants .OrderBy(g => g.SkillId, StringComparer.Ordinal) - .ThenBy(g => g.Amount) - .ToArray()); + .ThenBy(g => g.Amount)]); private static bool BundlesEqual(NormalizedBundle left, NormalizedBundle right) { diff --git a/server/NeonSprawl.Server/Game/Quests/PrototypeE7M3QuestFactionRules.cs b/server/NeonSprawl.Server/Game/Quests/PrototypeE7M3QuestFactionRules.cs index bc925b4..c30840b 100644 --- a/server/NeonSprawl.Server/Game/Quests/PrototypeE7M3QuestFactionRules.cs +++ b/server/NeonSprawl.Server/Game/Quests/PrototypeE7M3QuestFactionRules.cs @@ -161,18 +161,15 @@ public static class PrototypeE7M3QuestFactionRules private static NormalizedBundle NormalizeBundle(QuestRewardBundleRow bundle) => new( - bundle.ItemGrants + [.. bundle.ItemGrants .OrderBy(g => g.ItemId, StringComparer.Ordinal) - .ThenBy(g => g.Quantity) - .ToArray(), - bundle.SkillXpGrants + .ThenBy(g => g.Quantity)], + [.. bundle.SkillXpGrants .OrderBy(g => g.SkillId, StringComparer.Ordinal) - .ThenBy(g => g.Amount) - .ToArray(), - bundle.ReputationGrants + .ThenBy(g => g.Amount)], + [.. bundle.ReputationGrants .OrderBy(g => g.FactionId, StringComparer.Ordinal) - .ThenBy(g => g.Amount) - .ToArray()); + .ThenBy(g => g.Amount)]); private static bool BundlesEqual(NormalizedBundle left, NormalizedBundle right) { diff --git a/server/NeonSprawl.Server/Game/Quests/QuestDefinitionCatalogLoader.cs b/server/NeonSprawl.Server/Game/Quests/QuestDefinitionCatalogLoader.cs index 77e23c5..5d36b66 100644 --- a/server/NeonSprawl.Server/Game/Quests/QuestDefinitionCatalogLoader.cs +++ b/server/NeonSprawl.Server/Game/Quests/QuestDefinitionCatalogLoader.cs @@ -64,9 +64,8 @@ public static class QuestDefinitionCatalogLoader if (errors.Count > 0) ThrowIfAny(errors); - var jsonFiles = Directory.GetFiles(questsDirectory, "*_quests.json", SearchOption.TopDirectoryOnly) - .OrderBy(p => p, StringComparer.Ordinal) - .ToArray(); + string[] jsonFiles = [.. Directory.GetFiles(questsDirectory, "*_quests.json", SearchOption.TopDirectoryOnly) + .OrderBy(p => p, StringComparer.Ordinal)]; if (jsonFiles.Length == 0) errors.Add($"error: no *_quests.json files under {questsDirectory}"); diff --git a/server/NeonSprawl.Server/Game/Quests/QuestDefinitionRegistry.cs b/server/NeonSprawl.Server/Game/Quests/QuestDefinitionRegistry.cs index b6c2d3b..6c97d05 100644 --- a/server/NeonSprawl.Server/Game/Quests/QuestDefinitionRegistry.cs +++ b/server/NeonSprawl.Server/Game/Quests/QuestDefinitionRegistry.cs @@ -43,7 +43,7 @@ public sealed class QuestDefinitionRegistry(QuestDefinitionCatalog catalog) : IQ private static IReadOnlyList BuildDefinitionsInIdOrder(QuestDefinitionCatalog catalog) { - var ids = catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal).ToArray(); + string[] ids = [.. catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal)]; var list = new List(ids.Length); foreach (var id in ids) { diff --git a/server/NeonSprawl.Server/Game/Rewards/InMemoryRewardDeliveryStore.cs b/server/NeonSprawl.Server/Game/Rewards/InMemoryRewardDeliveryStore.cs index b9a89ed..2ecc621 100644 --- a/server/NeonSprawl.Server/Game/Rewards/InMemoryRewardDeliveryStore.cs +++ b/server/NeonSprawl.Server/Game/Rewards/InMemoryRewardDeliveryStore.cs @@ -27,9 +27,9 @@ public sealed class InMemoryRewardDeliveryStore : IRewardDeliveryStore eventsByKey[key] = deliveryEvent with { - GrantedItems = deliveryEvent.GrantedItems.ToArray(), - GrantedSkillXp = deliveryEvent.GrantedSkillXp.ToArray(), - GrantedReputation = deliveryEvent.GrantedReputation.ToArray(), + GrantedItems = [..deliveryEvent.GrantedItems], + GrantedSkillXp = [..deliveryEvent.GrantedSkillXp], + GrantedReputation = [..deliveryEvent.GrantedReputation], }; return true; } diff --git a/server/NeonSprawl.Server/Game/Skills/PostgresSkillProgressionBootstrap.cs b/server/NeonSprawl.Server/Game/Skills/PostgresSkillProgressionBootstrap.cs index 2eb3844..3753a37 100644 --- a/server/NeonSprawl.Server/Game/Skills/PostgresSkillProgressionBootstrap.cs +++ b/server/NeonSprawl.Server/Game/Skills/PostgresSkillProgressionBootstrap.cs @@ -1,3 +1,5 @@ +using System.Threading; + namespace NeonSprawl.Server.Game.Skills; /// Applies NEO-38 skill progression table DDL once per process. @@ -5,7 +7,7 @@ public static class PostgresSkillProgressionBootstrap { private static readonly string DdlRelativePath = Path.Combine("db", "migrations", "V003__player_skill_progression.sql"); - private static readonly object SchemaGate = new(); + private static readonly Lock SchemaGate = new(); private static int _schemaReady; diff --git a/server/NeonSprawl.Server/Game/Skills/SkillDefinitionCatalogLoader.cs b/server/NeonSprawl.Server/Game/Skills/SkillDefinitionCatalogLoader.cs index 4e9a35f..0e655f6 100644 --- a/server/NeonSprawl.Server/Game/Skills/SkillDefinitionCatalogLoader.cs +++ b/server/NeonSprawl.Server/Game/Skills/SkillDefinitionCatalogLoader.cs @@ -26,9 +26,8 @@ public static class SkillDefinitionCatalogLoader if (errors.Count > 0) ThrowIfAny(errors); - var jsonFiles = Directory.GetFiles(skillsDirectory, "*_skills.json", SearchOption.TopDirectoryOnly) - .OrderBy(p => p, StringComparer.Ordinal) - .ToArray(); + string[] jsonFiles = [.. Directory.GetFiles(skillsDirectory, "*_skills.json", SearchOption.TopDirectoryOnly) + .OrderBy(p => p, StringComparer.Ordinal)]; if (jsonFiles.Length == 0) errors.Add($"error: no *_skills.json files under {skillsDirectory}"); diff --git a/server/NeonSprawl.Server/Game/Skills/SkillDefinitionRegistry.cs b/server/NeonSprawl.Server/Game/Skills/SkillDefinitionRegistry.cs index fca38d2..f9cf20f 100644 --- a/server/NeonSprawl.Server/Game/Skills/SkillDefinitionRegistry.cs +++ b/server/NeonSprawl.Server/Game/Skills/SkillDefinitionRegistry.cs @@ -27,7 +27,7 @@ public sealed class SkillDefinitionRegistry(SkillDefinitionCatalog catalog) : IS /// public IReadOnlyList GetDefinitionsInIdOrder() { - var ids = catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal).ToArray(); + string[] ids = [.. catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal)]; var list = new List(ids.Length); foreach (var id in ids) { From e0877fea7fdebfcbe285d685c0e42afb7e80bedf Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 24 Jun 2026 20:41:40 -0400 Subject: [PATCH 06/18] NEO-146: use collection spread in EncounterProgressApi Apply IDE0305 to defeated-target ordering, assert stable sort in API tests, and document sorted defeatedTargetIds in Bruno. --- .../encounter-progress/Get encounter progress inactive.bru | 1 + .../Game/Encounters/EncounterProgressApiTests.cs | 2 ++ .../NeonSprawl.Server/Game/Encounters/EncounterProgressApi.cs | 2 +- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/bruno/neon-sprawl-server/encounter-progress/Get encounter progress inactive.bru b/bruno/neon-sprawl-server/encounter-progress/Get encounter progress inactive.bru index ccd29e6..cb72d06 100644 --- a/bruno/neon-sprawl-server/encounter-progress/Get encounter progress inactive.bru +++ b/bruno/neon-sprawl-server/encounter-progress/Get encounter progress inactive.bru @@ -7,6 +7,7 @@ meta { docs { NEO-108: fresh player row before engagement — inactive with empty defeatedTargetIds. Pre-request uses combat-targets dev fixture (also clears encounter progress for dev-local-1). + Non-empty defeatedTargetIds are returned in stable sorted order. } script:pre-request { diff --git a/server/NeonSprawl.Server.Tests/Game/Encounters/EncounterProgressApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Encounters/EncounterProgressApiTests.cs index d7dd7a4..c13ab43 100644 --- a/server/NeonSprawl.Server.Tests/Game/Encounters/EncounterProgressApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Encounters/EncounterProgressApiTests.cs @@ -124,6 +124,8 @@ public sealed class EncounterProgressApiTests PrototypeNpcRegistry.PrototypeNpcRangedId, ], row.DefeatedTargetIds); + Assert.True(row.DefeatedTargetIds.SequenceEqual( + row.DefeatedTargetIds.OrderBy(static id => id, StringComparer.Ordinal))); Assert.Null(row.CompletedAt); Assert.Null(row.RewardGrantSummary); var completionStore = factory.Services.GetRequiredService(); diff --git a/server/NeonSprawl.Server/Game/Encounters/EncounterProgressApi.cs b/server/NeonSprawl.Server/Game/Encounters/EncounterProgressApi.cs index 099a05c..97e25ea 100644 --- a/server/NeonSprawl.Server/Game/Encounters/EncounterProgressApi.cs +++ b/server/NeonSprawl.Server/Game/Encounters/EncounterProgressApi.cs @@ -72,7 +72,7 @@ public static class EncounterProgressApi { var hasProgress = progressStore.TryGetProgress(playerId, encounterId, out var progress); var defeatedTargetIds = hasProgress - ? progress.DefeatedNpcInstanceIds.OrderBy(static id => id, StringComparer.Ordinal).ToArray() + ? [.. progress.DefeatedNpcInstanceIds.OrderBy(static id => id, StringComparer.Ordinal)] : Array.Empty(); if (completionStore.IsCompleted(playerId, encounterId)) From b6b0b8753489cbfd7eebd2e63bb662ba746115d6 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 24 Jun 2026 20:44:30 -0400 Subject: [PATCH 07/18] NEO-146: fix remaining C# analyzer style warnings Apply using simplification, inline declarations, const locals, and Lock in PostgresDockerHelper; document RCS1118 and IDE0018 in style guide. --- .cursor/rules/csharp-style.md | 12 +++++++ ...tractOutcomePersistenceIntegrationTests.cs | 36 +++++++++---------- ...NodeInstancePersistenceIntegrationTests.cs | 6 ++-- .../PositionState/PostgresDockerHelper.cs | 3 +- ...uestProgressPersistenceIntegrationTests.cs | 4 +-- .../Quests/PrototypeE7M3QuestFactionRules.cs | 2 +- 6 files changed, 36 insertions(+), 27 deletions(-) diff --git a/.cursor/rules/csharp-style.md b/.cursor/rules/csharp-style.md index 1201727..335739c 100644 --- a/.cursor/rules/csharp-style.md +++ b/.cursor/rules/csharp-style.md @@ -81,6 +81,18 @@ return query.ToArray(); - **Keep braced `using (var …) { … }`** when the block has **multiple statements** (Arrange + Act + Assert inside one scope, seed loops, or any sequence that shares locals). - **Do not flatten** multi-statement blocks into `using var` at method scope when inner scopes reuse names like `store`, `registry`, or `scope` — that causes **CS0136** name clashes. Use distinct names (`seedStore`, `actScope`, `readStore`) or keep the braced block. +## Local `const` (RCS1118) + +- When a local is initialized from a **compile-time constant** (string literal, numeric literal, or `public const` field), declare it **`const`** instead of **`var`**. + +```csharp +const string questId = PrototypeE7M1QuestCatalogRules.GatherIntroQuestId; +``` + +## Inline declarations (IDE0018) + +- Prefer **inline initialization** over declare-then-assign when the analyzer suggests it, e.g. `var outcome = Operation(...)` and `TryGet(..., out var snapshot)` instead of separate upfront declarations. + ```csharp // Prefer — single disposable, single follow-on statement await using var conn = new NpgsqlConnection(cs); diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs index 11a386a..f8aaae4 100644 --- a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs @@ -85,25 +85,23 @@ public sealed class ContractOutcomePersistenceIntegrationTests(PostgresIntegrati [], RecordedAt); - using (var scope = Factory.Services.CreateScope()) - { - var instanceStore = scope.ServiceProvider.GetRequiredService(); - var outcomeStore = scope.ServiceProvider.GetRequiredService(); - Assert.True(instanceStore.TryCreateActive( - PlayerId, - InstanceId, - TemplateId, - SeedBucket, - IssuedAt, - out _)); - Assert.True(outcomeStore.TryAppend(outcomeRow)); - // Act - var duplicateId = outcomeStore.TryAppend(outcomeRow with { IdempotencyKey = "other-key" }); - var duplicateKey = outcomeStore.TryAppend(outcomeRow with { Id = "outcome-row-pg-002" }); - // Assert - Assert.False(duplicateId); - Assert.False(duplicateKey); - } + using var scope = Factory.Services.CreateScope(); + var instanceStore = scope.ServiceProvider.GetRequiredService(); + var outcomeStore = scope.ServiceProvider.GetRequiredService(); + Assert.True(instanceStore.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _)); + Assert.True(outcomeStore.TryAppend(outcomeRow)); + // Act + var duplicateId = outcomeStore.TryAppend(outcomeRow with { IdempotencyKey = "other-key" }); + var duplicateKey = outcomeStore.TryAppend(outcomeRow with { Id = "outcome-row-pg-002" }); + // Assert + Assert.False(duplicateId); + Assert.False(duplicateKey); } [RequirePostgresFact] diff --git a/server/NeonSprawl.Server.Tests/Game/Gathering/ResourceNodeInstancePersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Gathering/ResourceNodeInstancePersistenceIntegrationTests.cs index 29f5ec6..06fbac7 100644 --- a/server/NeonSprawl.Server.Tests/Game/Gathering/ResourceNodeInstancePersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Gathering/ResourceNodeInstancePersistenceIntegrationTests.cs @@ -57,14 +57,12 @@ public sealed class ResourceNodeInstancePersistenceIntegrationTests(PostgresInte } // Act - ResourceNodeInstanceMutationOutcome denyOutcome; - ResourceNodeInstanceSnapshot persisted; await using var secondFactory = new PostgresWebApplicationFactory(); using var actScope = secondFactory.Services.CreateScope(); var actRegistry = actScope.ServiceProvider.GetRequiredService(); var actStore = actScope.ServiceProvider.GetRequiredService(); - denyOutcome = ResourceNodeInstanceOperations.TryCommitSuccessfulGatherDecrement(AlphaId, actRegistry, actStore); - actStore.TryGetRemainingGathers(AlphaId, out persisted); + var denyOutcome = ResourceNodeInstanceOperations.TryCommitSuccessfulGatherDecrement(AlphaId, actRegistry, actStore); + actStore.TryGetRemainingGathers(AlphaId, out var persisted); // Assert Assert.Equal(ResourceNodeInstanceMutationKind.Denied, denyOutcome.Kind); diff --git a/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresDockerHelper.cs b/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresDockerHelper.cs index 50373da..d4b9c3a 100644 --- a/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresDockerHelper.cs +++ b/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresDockerHelper.cs @@ -1,4 +1,5 @@ using System.Diagnostics; +using System.Threading; using Npgsql; namespace NeonSprawl.Server.Tests.Game.PositionState; @@ -13,7 +14,7 @@ namespace NeonSprawl.Server.Tests.Game.PositionState; /// internal static class PostgresDockerHelper { - private static readonly object StateLock = new(); + private static readonly Lock StateLock = new(); private static Task? _ensureTask; private static bool _composeStartedByUs; private static string? _repoRoot; diff --git a/server/NeonSprawl.Server.Tests/Game/Quests/PlayerQuestProgressPersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Quests/PlayerQuestProgressPersistenceIntegrationTests.cs index 60f43fa..3f7398b 100644 --- a/server/NeonSprawl.Server.Tests/Game/Quests/PlayerQuestProgressPersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Quests/PlayerQuestProgressPersistenceIntegrationTests.cs @@ -22,7 +22,7 @@ public sealed class PlayerQuestProgressPersistenceIntegrationTests(PostgresInteg // Arrange await ResetQuestProgressTableAsync(); const int concurrentCalls = 8; - var questId = PrototypeE7M1QuestCatalogRules.GatherIntroQuestId; + const string questId = PrototypeE7M1QuestCatalogRules.GatherIntroQuestId; // Act var results = await Task.WhenAll( @@ -93,7 +93,7 @@ public sealed class PlayerQuestProgressPersistenceIntegrationTests(PostgresInteg { // Arrange await ResetQuestProgressTableAsync(); - var questId = PrototypeE7M1QuestCatalogRules.GatherIntroQuestId; + const string questId = PrototypeE7M1QuestCatalogRules.GatherIntroQuestId; // Act using (var firstScope = Factory.Services.CreateScope()) diff --git a/server/NeonSprawl.Server/Game/Quests/PrototypeE7M3QuestFactionRules.cs b/server/NeonSprawl.Server/Game/Quests/PrototypeE7M3QuestFactionRules.cs index c30840b..141d462 100644 --- a/server/NeonSprawl.Server/Game/Quests/PrototypeE7M3QuestFactionRules.cs +++ b/server/NeonSprawl.Server/Game/Quests/PrototypeE7M3QuestFactionRules.cs @@ -107,7 +107,7 @@ public static class PrototypeE7M3QuestFactionRules /// Returns a human-readable error if grid contract quest shape fails, otherwise . public static string? TryGetGridContractShapeError(IReadOnlyDictionary rowsById) { - var qid = PrototypeE7M1QuestCatalogRules.GridContractQuestId; + const string qid = PrototypeE7M1QuestCatalogRules.GridContractQuestId; if (!rowsById.TryGetValue(qid, out var row)) return $"error: missing quest '{qid}'"; From b519f4e9eb2e027383f35135ecee2ec76be5e6cb Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 24 Jun 2026 20:46:23 -0400 Subject: [PATCH 08/18] NEO-146: fix mutation outcome docs and null-check style Add summary and Kind param XML docs on mutation outcome records, simplify PostgresDockerHelper null guard with ?? throw, and document RCS1139/IDE0270. --- .cursor/rules/csharp-style.md | 3 ++- .../Game/PositionState/PostgresDockerHelper.cs | 7 ++----- .../Game/Gathering/ResourceNodeInstanceMutationOutcome.cs | 2 ++ .../Game/Items/PlayerInventoryMutationOutcome.cs | 2 ++ 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/.cursor/rules/csharp-style.md b/.cursor/rules/csharp-style.md index 335739c..9a9c298 100644 --- a/.cursor/rules/csharp-style.md +++ b/.cursor/rules/csharp-style.md @@ -133,8 +133,9 @@ var store = secondScope.ServiceProvider.GetRequiredService(); // ## Documentation - Use **`///` XML doc comments** on public APIs (types and members) when behavior or contracts are not obvious. +- **Primary-constructor records:** include a **``** plus **``** for every constructor parameter (Roslynator RCS1139 / RCS1263). Do not leave orphaned `` lines without a summary on the type. -## Test project layout (`*.Tests`) +## Null checks (IDE0270) (`*.Tests`) - **Mirror the server project:** place test types under the same relative path as the production code they exercise (e.g. `NeonSprawl.Server/Game/PositionState/PositionStateApi.cs` → `NeonSprawl.Server.Tests/Game/PositionState/PositionStateApiTests.cs`). - Use a namespace that matches the folder tree under the test assembly root, e.g. `NeonSprawl.Server.Tests.Game.PositionState` for files in `Game/PositionState/`. diff --git a/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresDockerHelper.cs b/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresDockerHelper.cs index d4b9c3a..9ed5627 100644 --- a/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresDockerHelper.cs +++ b/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresDockerHelper.cs @@ -85,12 +85,9 @@ internal static class PostgresDockerHelper return; } - var root = FindDockerComposeRoot(); - if (root is null) - { - throw new InvalidOperationException( + var root = FindDockerComposeRoot() + ?? throw new InvalidOperationException( "Postgres is not reachable and docker-compose.yml was not found by walking up from the test output directory."); - } await RunDockerComposeAsync(root, "compose up -d", CancellationToken.None).ConfigureAwait(false); diff --git a/server/NeonSprawl.Server/Game/Gathering/ResourceNodeInstanceMutationOutcome.cs b/server/NeonSprawl.Server/Game/Gathering/ResourceNodeInstanceMutationOutcome.cs index 8c1f0bb..351025b 100644 --- a/server/NeonSprawl.Server/Game/Gathering/ResourceNodeInstanceMutationOutcome.cs +++ b/server/NeonSprawl.Server/Game/Gathering/ResourceNodeInstanceMutationOutcome.cs @@ -10,6 +10,8 @@ public enum ResourceNodeInstanceMutationKind Applied, } +/// Result of (NEO-61). +/// Applied or denied. /// Populated when is . /// Authoritative snapshot after apply, or current snapshot on depletion deny; null for unknown node. public readonly record struct ResourceNodeInstanceMutationOutcome( diff --git a/server/NeonSprawl.Server/Game/Items/PlayerInventoryMutationOutcome.cs b/server/NeonSprawl.Server/Game/Items/PlayerInventoryMutationOutcome.cs index edec38b..645bdc2 100644 --- a/server/NeonSprawl.Server/Game/Items/PlayerInventoryMutationOutcome.cs +++ b/server/NeonSprawl.Server/Game/Items/PlayerInventoryMutationOutcome.cs @@ -13,6 +13,8 @@ public enum PlayerInventoryMutationKind Applied, } +/// Result of or (NEO-54). +/// Applied, denied, or store missing. /// Populated when is . /// Authoritative snapshot after apply, or unchanged snapshot on deny; null when store missing. public readonly record struct PlayerInventoryMutationOutcome( From a4f37db594c6446498d657b5227b4a53a33182b5 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 24 Jun 2026 20:46:27 -0400 Subject: [PATCH 09/18] NEO-146: restore test layout section in csharp-style guide Fix IDE0270 section that accidentally overwrote the test project layout heading. --- .cursor/rules/csharp-style.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.cursor/rules/csharp-style.md b/.cursor/rules/csharp-style.md index 9a9c298..641404f 100644 --- a/.cursor/rules/csharp-style.md +++ b/.cursor/rules/csharp-style.md @@ -135,7 +135,16 @@ var store = secondScope.ServiceProvider.GetRequiredService(); // - Use **`///` XML doc comments** on public APIs (types and members) when behavior or contracts are not obvious. - **Primary-constructor records:** include a **``** plus **``** for every constructor parameter (Roslynator RCS1139 / RCS1263). Do not leave orphaned `` lines without a summary on the type. -## Null checks (IDE0270) (`*.Tests`) +## Null checks (IDE0270) + +- Prefer **`?? throw`** over `if (x is null) { throw … }` when assigning a non-null local from a nullable expression. + +```csharp +var root = FindDockerComposeRoot() + ?? throw new InvalidOperationException("…"); +``` + +## Test project layout (`*.Tests`) - **Mirror the server project:** place test types under the same relative path as the production code they exercise (e.g. `NeonSprawl.Server/Game/PositionState/PositionStateApi.cs` → `NeonSprawl.Server.Tests/Game/PositionState/PositionStateApiTests.cs`). - Use a namespace that matches the folder tree under the test assembly root, e.g. `NeonSprawl.Server.Tests.Game.PositionState` for files in `Game/PositionState/`. From 01e82f38f664203ddeb104c5b4aaad8d289c0b0f Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 24 Jun 2026 20:47:40 -0400 Subject: [PATCH 10/18] NEO-146: fix record struct docs and collection spread in tests Replace param XML on record struct mutation outcomes with summary cref docs to avoid Roslynator RCS1263 false positives; apply IDE0305 in RewardRouterOperationsTests and update style guide guidance. --- .cursor/rules/csharp-style.md | 2 +- .../Game/Rewards/RewardRouterOperationsTests.cs | 2 +- .../Gathering/ResourceNodeInstanceMutationOutcome.cs | 9 +++++---- .../Game/Items/PlayerInventoryMutationOutcome.cs | 9 +++++---- 4 files changed, 12 insertions(+), 10 deletions(-) diff --git a/.cursor/rules/csharp-style.md b/.cursor/rules/csharp-style.md index 641404f..adf9152 100644 --- a/.cursor/rules/csharp-style.md +++ b/.cursor/rules/csharp-style.md @@ -133,7 +133,7 @@ var store = secondScope.ServiceProvider.GetRequiredService(); // ## Documentation - Use **`///` XML doc comments** on public APIs (types and members) when behavior or contracts are not obvious. -- **Primary-constructor records:** include a **``** plus **``** for every constructor parameter (Roslynator RCS1139 / RCS1263). Do not leave orphaned `` lines without a summary on the type. +- **Primary-constructor records:** include a **``** on the type. Prefer **``** in the summary for positional parameters. Do **not** use **``** on **`record struct`** primary constructors — Roslynator reports false-positive **RCS1263** ([roslynator#1730](https://github.com/dotnet/roslynator/issues/1730)). **`record class`** / **`class`** primary constructors may use `` when the analyzer accepts them. ## Null checks (IDE0270) diff --git a/server/NeonSprawl.Server.Tests/Game/Rewards/RewardRouterOperationsTests.cs b/server/NeonSprawl.Server.Tests/Game/Rewards/RewardRouterOperationsTests.cs index fa40ef2..673f377 100644 --- a/server/NeonSprawl.Server.Tests/Game/Rewards/RewardRouterOperationsTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Rewards/RewardRouterOperationsTests.cs @@ -803,7 +803,7 @@ public sealed class RewardRouterOperationsTests var filtered = rows.Where(r => r.PlayerId == normalized).ToList(); if (limit is > 0) { - return filtered.Take(limit.Value).ToList(); + return [.. filtered.Take(limit.Value)]; } return filtered; diff --git a/server/NeonSprawl.Server/Game/Gathering/ResourceNodeInstanceMutationOutcome.cs b/server/NeonSprawl.Server/Game/Gathering/ResourceNodeInstanceMutationOutcome.cs index 351025b..2b714aa 100644 --- a/server/NeonSprawl.Server/Game/Gathering/ResourceNodeInstanceMutationOutcome.cs +++ b/server/NeonSprawl.Server/Game/Gathering/ResourceNodeInstanceMutationOutcome.cs @@ -10,10 +10,11 @@ public enum ResourceNodeInstanceMutationKind Applied, } -/// Result of (NEO-61). -/// Applied or denied. -/// Populated when is . -/// Authoritative snapshot after apply, or current snapshot on depletion deny; null for unknown node. +/// +/// Result of (NEO-61). +/// is populated when is . +/// is the authoritative snapshot after apply, or the current snapshot on depletion deny; null for unknown node. +/// public readonly record struct ResourceNodeInstanceMutationOutcome( ResourceNodeInstanceMutationKind Kind, string? ReasonCode, diff --git a/server/NeonSprawl.Server/Game/Items/PlayerInventoryMutationOutcome.cs b/server/NeonSprawl.Server/Game/Items/PlayerInventoryMutationOutcome.cs index 645bdc2..a224a5d 100644 --- a/server/NeonSprawl.Server/Game/Items/PlayerInventoryMutationOutcome.cs +++ b/server/NeonSprawl.Server/Game/Items/PlayerInventoryMutationOutcome.cs @@ -13,10 +13,11 @@ public enum PlayerInventoryMutationKind Applied, } -/// Result of or (NEO-54). -/// Applied, denied, or store missing. -/// Populated when is . -/// Authoritative snapshot after apply, or unchanged snapshot on deny; null when store missing. +/// +/// Result of or (NEO-54). +/// is populated when is . +/// is the authoritative snapshot after apply, or unchanged snapshot on deny; null when store missing. +/// public readonly record struct PlayerInventoryMutationOutcome( PlayerInventoryMutationKind Kind, string? ReasonCode, From a9bc1acaa88464a56aaab5c8b31df95d3004e156 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 24 Jun 2026 20:51:04 -0400 Subject: [PATCH 11/18] NEO-146: add re-review after cleanup and review follow-up --- docs/reviews/2026-06-24-NEO-146-r2.md | 73 +++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 docs/reviews/2026-06-24-NEO-146-r2.md diff --git a/docs/reviews/2026-06-24-NEO-146-r2.md b/docs/reviews/2026-06-24-NEO-146-r2.md new file mode 100644 index 0000000..7afb6c1 --- /dev/null +++ b/docs/reviews/2026-06-24-NEO-146-r2.md @@ -0,0 +1,73 @@ +# Code review (re-review) — NEO-146 (E7M4-03 contract instance + outcome stores) + +**Date:** 2026-06-24 +**Scope:** Branch `NEO-146-e7m4-03-contract-instance-outcome-stores` — commits `82c4594`…`01e82f3` (10 commits); supersedes [2026-06-24-NEO-146.md](2026-06-24-NEO-146.md) initial review +**Base:** `main` + +## Verdict + +**Approve with nits** + +## Summary + +Follow-up review after the initial NEO-146 review and a broad C# style/analyzer cleanup pass. All prior **Suggestions** and actionable **Nits** from the first review are addressed: decomposition alignment updated, Postgres one-active race + deny-path tests added, duplicate instance id unit test, FK-safe `PostgresContractOutcomeStore.TryAppend`, `MakeInstanceKey` / player-level locking wired in the in-memory instance store, README orchestrator ordering documented, and plan reconciliation brought current. Subsequent commits apply collection spread, `await using` / `Lock` schema gates, and related analyzer conventions repo-wide (documented in `.cursor/rules/csharp-style.md`). Contract store behavior matches the plan; `dotnet test` passes **867** tests with **0** build warnings. Risk remains low — infrastructure-only, no HTTP surface. + +## Documentation checked + +| Document | Alignment | +|----------|-----------| +| `docs/plans/NEO-146-implementation-plan.md` | **Matches** — AC complete; reconciliation includes Bruno smoke, `MakeInstanceKey` locking, Postgres clear adopted, review follow-up tests. Test count says `866`; suite now **867** (minor drift). | +| `docs/plans/E7M4-pre-production-backlog.md` (E7M4-03) | **Matches** | +| `docs/decomposition/modules/E7_M4_ContractMissionGenerator.md` | **Matches** — runtime stores paragraph (NEO-146). | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E7M4-03 / NEO-146 stores row added. | +| `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E7M4-03 / NEO-146 note added. | +| `.cursor/rules/csharp-style.md` | **Matches** — new sections document collection spread, `using`/`await using`, `Lock` schema gates, CA1859, null-check style adopted in cleanup commits. | +| Full-stack epic decomposition | **N/A** — infrastructure-only. | + +## Prior review resolution + +| Item (from `2026-06-24-NEO-146.md`) | Status | +|-------------------------------------|--------| +| Decomposition tracking (alignment + register) | **Done** | +| Postgres distinct-instance-id one-active race test | **Done** — `TryCreateActive_ShouldAllowOnlyOneActive_WhenDistinctInstanceIdsRace` | +| Postgres outcome deny paths + idempotent complete | **Done** | +| In-memory duplicate instance id test | **Done** | +| Postgres FK parity on outcome append | **Done** — catch + README | +| `MakeInstanceKey` wiring / plan trim | **Done** | +| Bruno + plan reconciliation | **Done** | +| Postgres `TryClear*` plan status | **Done** — adopted | + +## Blocking issues + +None. + +## Suggestions + +1. **PR / commit scope for future stories** — Commits `b455ff5`…`01e82f3` touch ~50 files outside `Game/Contracts/` (catalog loaders, integration-test `using` layout, bootstrap `Lock` gates, mutation-outcome docs). Mechanical and green, but it widens review/merge-conflict surface for an infrastructure ticket. Consider a separate `chore:` PR for repo-wide analyzer/style passes when the diff is this large. + +2. **In-memory outcome instance guard (optional parity)** — Postgres `TryAppend` returns `false` when `contract_instance` is missing (FK); in-memory still accepts a normalized row without checking instance existence. README documents Postgres ordering; orchestrators (NEO-149) will create the instance first. If strict cross-backend parity matters before NEO-149, add an optional `IContractInstanceStore` lookup in `InMemoryContractOutcomeStore.TryAppend` or a shared guard helper — otherwise document the asymmetry explicitly in README (one line under in-memory storage). + +## Nits + +- Nit: Plan reconciliation cites **866** tests; re-review run reports **867** — update the count on next doc touch. + +- Nit: `ContractInstanceReasonCodes` remains unused until NEO-147 orchestrators — expected deferral; no action in E7M4-03. + +- Nit: `InMemoryContractInstanceStore` uses `PlayerLockKey` for create/active reads and `MakeInstanceKey` for get/complete/clear on a specific instance. One-active policy holds under analysis (create path serializes on player lock; active index checked before insert), but the split lock granularity is slightly harder to reason about than a single player-level mutex — acceptable for prototype; no change required unless concurrency tests expand. + +## Verification + +Commands run during re-review (all green): + +```bash +dotnet build NeonSprawl.sln +dotnet test NeonSprawl.sln +``` + +Optional Postgres integration (when `ConnectionStrings__NeonSprawl` is set): + +```bash +dotnet test NeonSprawl.sln --filter "FullyQualifiedName~ContractInstancePersistence|FullyQualifiedName~ContractOutcomePersistence" +``` + +Merge-ready: prior review feedback is closed; remaining items are process/parity nits only. From b087c03a47ba0a8897ca8bbb72cc2ad523aff7c9 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 24 Jun 2026 20:52:49 -0400 Subject: [PATCH 12/18] NEO-146: address re-review in-memory outcome guard parity InMemoryContractOutcomeStore checks IContractInstanceStore.TryGet before append; add unit test, README/plan updates, and close r2 review items. --- docs/plans/NEO-146-implementation-plan.md | 3 +- docs/reviews/2026-06-24-NEO-146-r2.md | 10 ++-- .../InMemoryContractOutcomeStoreTests.cs | 48 +++++++++++++++++-- .../Contracts/InMemoryContractOutcomeStore.cs | 7 ++- server/README.md | 4 +- 5 files changed, 59 insertions(+), 13 deletions(-) diff --git a/docs/plans/NEO-146-implementation-plan.md b/docs/plans/NEO-146-implementation-plan.md index bba779f..2083e50 100644 --- a/docs/plans/NEO-146-implementation-plan.md +++ b/docs/plans/NEO-146-implementation-plan.md @@ -68,7 +68,8 @@ Durable per-player **`ContractInstance`** rows (active/completed) and append-onl - **Types:** `ContractInstanceStatus`, `ContractInstanceState`, `ContractInstanceIds`, `ContractInstanceReasonCodes`, `ContractOutcomeRow`, `ContractOutcomeIds`. - **Stores:** `IContractInstanceStore` / `IContractOutcomeStore`; in-memory + Postgres (`V011`, `V012`) when connection string set; partial unique index + app-level one-active enforcement. - **DI:** `AddContractInstanceStores` in `Program.cs`; in-memory override in `InMemoryWebApplicationFactory`. -- **Tests:** `InMemoryContractInstanceStoreTests`, `InMemoryContractOutcomeStoreTests`, Postgres persistence integration tests; host DI smoke (`866` tests green). +- **Tests:** `InMemoryContractInstanceStoreTests`, `InMemoryContractOutcomeStoreTests`, Postgres persistence integration tests; host DI smoke (`867` tests green). +- **In-memory outcome guard:** `InMemoryContractOutcomeStore.TryAppend` denies when **`IContractInstanceStore.TryGet`** is false (Postgres FK parity). - **Docs:** `server/README.md` contract instance + outcome store sections. - **Bruno:** `bruno/neon-sprawl-server/contract-stores/` health smoke (startup-only; no contract HTTP in E7M4-03). diff --git a/docs/reviews/2026-06-24-NEO-146-r2.md b/docs/reviews/2026-06-24-NEO-146-r2.md index 7afb6c1..d8350dc 100644 --- a/docs/reviews/2026-06-24-NEO-146-r2.md +++ b/docs/reviews/2026-06-24-NEO-146-r2.md @@ -16,7 +16,7 @@ Follow-up review after the initial NEO-146 review and a broad C# style/analyzer | Document | Alignment | |----------|-----------| -| `docs/plans/NEO-146-implementation-plan.md` | **Matches** — AC complete; reconciliation includes Bruno smoke, `MakeInstanceKey` locking, Postgres clear adopted, review follow-up tests. Test count says `866`; suite now **867** (minor drift). | +| `docs/plans/NEO-146-implementation-plan.md` | **Matches** — AC complete; reconciliation includes Bruno smoke, `MakeInstanceKey` locking, Postgres clear adopted, review follow-up tests. Test count **867**. | | `docs/plans/E7M4-pre-production-backlog.md` (E7M4-03) | **Matches** | | `docs/decomposition/modules/E7_M4_ContractMissionGenerator.md` | **Matches** — runtime stores paragraph (NEO-146). | | `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E7M4-03 / NEO-146 stores row added. | @@ -43,13 +43,13 @@ None. ## Suggestions -1. **PR / commit scope for future stories** — Commits `b455ff5`…`01e82f3` touch ~50 files outside `Game/Contracts/` (catalog loaders, integration-test `using` layout, bootstrap `Lock` gates, mutation-outcome docs). Mechanical and green, but it widens review/merge-conflict surface for an infrastructure ticket. Consider a separate `chore:` PR for repo-wide analyzer/style passes when the diff is this large. +1. ~~**PR / commit scope for future stories**~~ — **Intentional.** Repo-wide analyzer/style cleanup (`b455ff5`…`01e82f3`) was bundled on this branch by author request; no split `chore:` PR for E7M4-03. -2. **In-memory outcome instance guard (optional parity)** — Postgres `TryAppend` returns `false` when `contract_instance` is missing (FK); in-memory still accepts a normalized row without checking instance existence. README documents Postgres ordering; orchestrators (NEO-149) will create the instance first. If strict cross-backend parity matters before NEO-149, add an optional `IContractInstanceStore` lookup in `InMemoryContractOutcomeStore.TryAppend` or a shared guard helper — otherwise document the asymmetry explicitly in README (one line under in-memory storage). +2. ~~**In-memory outcome instance guard (optional parity)**~~ — **Done.** `InMemoryContractOutcomeStore` injects **`IContractInstanceStore`** and denies **`TryAppend`** when **`TryGet`** is false; unit test + README/plan updated (Postgres FK parity). ## Nits -- Nit: Plan reconciliation cites **866** tests; re-review run reports **867** — update the count on next doc touch. +- ~~Nit: Plan reconciliation cites **866** tests; re-review run reports **867**~~ — **Done.** Plan reconciliation updated to **867**. - Nit: `ContractInstanceReasonCodes` remains unused until NEO-147 orchestrators — expected deferral; no action in E7M4-03. @@ -70,4 +70,4 @@ Optional Postgres integration (when `ConnectionStrings__NeonSprawl` is set): dotnet test NeonSprawl.sln --filter "FullyQualifiedName~ContractInstancePersistence|FullyQualifiedName~ContractOutcomePersistence" ``` -Merge-ready: prior review feedback is closed; remaining items are process/parity nits only. +Merge-ready: all suggestions addressed or intentionally accepted; remaining nits are expected deferrals only. diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs index 73f0787..9acbaf2 100644 --- a/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs @@ -1,4 +1,6 @@ +using Microsoft.Extensions.Options; using NeonSprawl.Server.Game.Contracts; +using NeonSprawl.Server.Game.PositionState; using NeonSprawl.Server.Game.Rewards; using Xunit; @@ -9,13 +11,17 @@ public sealed class InMemoryContractOutcomeStoreTests private const string PlayerId = "dev-local-1"; private const string InstanceId = "prototype_contract_instance_001"; private const string OutcomeRowId = "outcome-row-001"; + private const string TemplateId = "prototype_contract_clear_combat_pocket"; + private const string SeedBucket = "2026-06-22"; + private static readonly DateTimeOffset IssuedAt = new(2026, 6, 22, 10, 0, 0, TimeSpan.Zero); private static readonly DateTimeOffset RecordedAt = new(2026, 6, 22, 12, 0, 0, TimeSpan.Zero); [Fact] public void TryAppend_ShouldPersistAndQueryByInstanceAndIdempotencyKey() { // Arrange - var store = new InMemoryContractOutcomeStore(); + var (instanceStore, store) = CreateStores(); + SeedActiveInstance(instanceStore); var row = CreatePrototypeOutcome(); // Act var appended = store.TryAppend(row); @@ -36,7 +42,8 @@ public sealed class InMemoryContractOutcomeStoreTests public void TryAppend_ShouldDenyDuplicateRowId() { // Arrange - var store = new InMemoryContractOutcomeStore(); + var (instanceStore, store) = CreateStores(); + SeedActiveInstance(instanceStore); var row = CreatePrototypeOutcome(); Assert.True(store.TryAppend(row)); // Act @@ -49,7 +56,8 @@ public sealed class InMemoryContractOutcomeStoreTests public void TryAppend_ShouldDenyDuplicateIdempotencyKey() { // Arrange - var store = new InMemoryContractOutcomeStore(); + var (instanceStore, store) = CreateStores(); + SeedActiveInstance(instanceStore); var row = CreatePrototypeOutcome(); Assert.True(store.TryAppend(row)); // Act @@ -62,7 +70,8 @@ public sealed class InMemoryContractOutcomeStoreTests public void TryAppend_ShouldFailClosed_WhenRequiredFieldsEmpty() { // Arrange - var store = new InMemoryContractOutcomeStore(); + var (instanceStore, store) = CreateStores(); + SeedActiveInstance(instanceStore); var invalidRow = CreatePrototypeOutcome() with { PlayerId = " " }; // Act var appended = store.TryAppend(invalidRow); @@ -71,6 +80,37 @@ public sealed class InMemoryContractOutcomeStoreTests Assert.Empty(store.GetOutcomesForInstance(InstanceId)); } + [Fact] + public void TryAppend_ShouldReturnFalse_WhenContractInstanceMissing() + { + // Arrange + var (_, store) = CreateStores(); + var row = CreatePrototypeOutcome(); + // Act + var appended = store.TryAppend(row); + // Assert + Assert.False(appended); + Assert.Empty(store.GetOutcomesForInstance(InstanceId)); + } + + private static (InMemoryContractInstanceStore InstanceStore, InMemoryContractOutcomeStore OutcomeStore) CreateStores() + { + var options = Options.Create(new GamePositionOptions { DevPlayerId = PlayerId }); + var instanceStore = new InMemoryContractInstanceStore(options); + return (instanceStore, new InMemoryContractOutcomeStore(instanceStore)); + } + + private static void SeedActiveInstance(InMemoryContractInstanceStore instanceStore) + { + Assert.True(instanceStore.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _)); + } + private static ContractOutcomeRow CreatePrototypeOutcome() { var idempotencyKey = ContractOutcomeIds.MakeIdempotencyKey(PlayerId, InstanceId); diff --git a/server/NeonSprawl.Server/Game/Contracts/InMemoryContractOutcomeStore.cs b/server/NeonSprawl.Server/Game/Contracts/InMemoryContractOutcomeStore.cs index 0e502df..7340c06 100644 --- a/server/NeonSprawl.Server/Game/Contracts/InMemoryContractOutcomeStore.cs +++ b/server/NeonSprawl.Server/Game/Contracts/InMemoryContractOutcomeStore.cs @@ -4,7 +4,7 @@ using NeonSprawl.Server.Game.Rewards; namespace NeonSprawl.Server.Game.Contracts; /// Thread-safe in-memory append-only contract outcome audit log (NEO-146). -public sealed class InMemoryContractOutcomeStore : IContractOutcomeStore +public sealed class InMemoryContractOutcomeStore(IContractInstanceStore instanceStore) : IContractOutcomeStore { private readonly ConcurrentDictionary rowsById = new(StringComparer.Ordinal); @@ -20,6 +20,11 @@ public sealed class InMemoryContractOutcomeStore : IContractOutcomeStore return false; } + if (!instanceStore.TryGet(normalized.PlayerId, normalized.ContractInstanceId, out _)) + { + return false; + } + lock (idLocks.GetOrAdd(normalized.Id, _ => new object())) { if (rowsById.ContainsKey(normalized.Id) || diff --git a/server/README.md b/server/README.md index 1f79191..dc42b43 100644 --- a/server/README.md +++ b/server/README.md @@ -292,12 +292,12 @@ Append-only contract completion audit rows live in **`IContractOutcomeStore`**. **Store interface methods:** -- **`TryAppend`** — first row id or idempotency key returns `true`; duplicates return `false`. Missing **`contract_instance`** row (Postgres FK) also returns `false` — orchestrators (NEO-149) must create the instance before appending an outcome. +- **`TryAppend`** — first row id or idempotency key returns `true`; duplicates return `false`. Missing **`contract_instance`** row returns `false` (Postgres FK; in-memory checks **`IContractInstanceStore.TryGet`**) — orchestrators (NEO-149) must create the instance before appending an outcome. - **`TryGetByIdempotencyKey`** — read one outcome for delivery replay / audit tests. - **`GetOutcomesForInstance`** — list outcomes for one contract instance id (ordered by recorded time). - **`TryClearForInstance`** — dev fixture only. -**Storage:** in-memory when Postgres is unset; **`PostgresContractOutcomeStore`** appends to **`contract_outcome`** ([`V012__contract_outcome.sql`](../db/migrations/V012__contract_outcome.sql)) with JSONB grant snapshots when configured. Plan: [NEO-146 implementation plan](../../docs/plans/NEO-146-implementation-plan.md). +**Storage:** in-memory when Postgres is unset (**`InMemoryContractOutcomeStore`** requires a matching instance row in **`IContractInstanceStore`**); **`PostgresContractOutcomeStore`** appends to **`contract_outcome`** ([`V012__contract_outcome.sql`](../db/migrations/V012__contract_outcome.sql)) with JSONB grant snapshots when configured. Plan: [NEO-146 implementation plan](../../docs/plans/NEO-146-implementation-plan.md). ## Quest definitions (NEO-115) From 7740d2145fc443bc3ea83529b7f8184c357d8321 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 24 Jun 2026 20:58:17 -0400 Subject: [PATCH 13/18] NEO-146: address Bugbot findings on contract outcome stores Ensure instance schema before outcome DDL, require matching player/instance on Postgres append, catch unique violations on concurrent duplicates, and add parity plus concurrency integration tests. --- docs/plans/NEO-146-implementation-plan.md | 4 +- ...tractOutcomePersistenceIntegrationTests.cs | 98 +++++++++++++++++++ .../InMemoryContractOutcomeStoreTests.cs | 14 +++ .../PostgresContractOutcomeBootstrap.cs | 2 + .../Contracts/PostgresContractOutcomeStore.cs | 9 +- server/README.md | 2 +- 6 files changed, 124 insertions(+), 5 deletions(-) diff --git a/docs/plans/NEO-146-implementation-plan.md b/docs/plans/NEO-146-implementation-plan.md index 2083e50..78be83c 100644 --- a/docs/plans/NEO-146-implementation-plan.md +++ b/docs/plans/NEO-146-implementation-plan.md @@ -68,8 +68,8 @@ Durable per-player **`ContractInstance`** rows (active/completed) and append-onl - **Types:** `ContractInstanceStatus`, `ContractInstanceState`, `ContractInstanceIds`, `ContractInstanceReasonCodes`, `ContractOutcomeRow`, `ContractOutcomeIds`. - **Stores:** `IContractInstanceStore` / `IContractOutcomeStore`; in-memory + Postgres (`V011`, `V012`) when connection string set; partial unique index + app-level one-active enforcement. - **DI:** `AddContractInstanceStores` in `Program.cs`; in-memory override in `InMemoryWebApplicationFactory`. -- **Tests:** `InMemoryContractInstanceStoreTests`, `InMemoryContractOutcomeStoreTests`, Postgres persistence integration tests; host DI smoke (`867` tests green). -- **In-memory outcome guard:** `InMemoryContractOutcomeStore.TryAppend` denies when **`IContractInstanceStore.TryGet`** is false (Postgres FK parity). +- **Tests:** `InMemoryContractInstanceStoreTests`, `InMemoryContractOutcomeStoreTests`, Postgres persistence integration tests; host DI smoke (`871` tests green). +- **In-memory outcome guard:** `InMemoryContractOutcomeStore.TryAppend` denies when **`IContractInstanceStore.TryGet`** is false (Postgres FK + player/instance SQL guard parity). - **Docs:** `server/README.md` contract instance + outcome store sections. - **Bruno:** `bruno/neon-sprawl-server/contract-stores/` health smoke (startup-only; no contract HTTP in E7M4-03). diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs index f8aaae4..3bd6c0a 100644 --- a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs @@ -129,6 +129,104 @@ public sealed class ContractOutcomePersistenceIntegrationTests(PostgresIntegrati Assert.False(appended); } + [RequirePostgresFact] + public async Task TryAppend_ShouldReturnFalse_WhenPlayerIdDoesNotMatchInstanceOwner() + { + // Arrange + await ResetContractTablesAsync(); + const string otherPlayerId = "other-player-pg-001"; + var idempotencyKey = ContractOutcomeIds.MakeIdempotencyKey(otherPlayerId, InstanceId); + var outcomeRow = new ContractOutcomeRow( + OutcomeRowId, + InstanceId, + otherPlayerId, + idempotencyKey, + [new RewardItemGrantApplied("scrap_metal_bulk", 5)], + [], + [], + RecordedAt); + using (var scope = Factory.Services.CreateScope()) + { + var instanceStore = scope.ServiceProvider.GetRequiredService(); + var outcomeStore = scope.ServiceProvider.GetRequiredService(); + Assert.True(instanceStore.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _)); + await SeedPostgresPlayerAsync(otherPlayerId); + // Act + var appended = outcomeStore.TryAppend(outcomeRow); + // Assert + Assert.False(appended); + Assert.Empty(outcomeStore.GetOutcomesForInstance(InstanceId)); + } + } + + [RequirePostgresFact] + public async Task TryAppend_ShouldAllowOnlyOneSuccess_WhenConcurrentDuplicateRowsRace() + { + // Arrange + await ResetContractTablesAsync(); + var idempotencyKey = ContractOutcomeIds.MakeIdempotencyKey(PlayerId, InstanceId); + var outcomeRow = new ContractOutcomeRow( + OutcomeRowId, + InstanceId, + PlayerId, + idempotencyKey, + [new RewardItemGrantApplied("scrap_metal_bulk", 5)], + [], + [], + RecordedAt); + using (var scope = Factory.Services.CreateScope()) + { + var instanceStore = scope.ServiceProvider.GetRequiredService(); + Assert.True(instanceStore.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _)); + } + + // Act + var results = await Task.WhenAll( + Enumerable.Range(0, 8).Select(_ => Task.Run(() => + { + using var scope = Factory.Services.CreateScope(); + var outcomeStore = scope.ServiceProvider.GetRequiredService(); + return outcomeStore.TryAppend(outcomeRow); + }))); + + // Assert + Assert.Equal(1, results.Count(static r => r)); + Assert.Equal(7, results.Count(static r => !r)); + } + + private static async Task SeedPostgresPlayerAsync(string playerId) + { + var cs = Environment.GetEnvironmentVariable("ConnectionStrings__NeonSprawl"); + if (string.IsNullOrWhiteSpace(cs)) + { + throw new InvalidOperationException("ConnectionStrings__NeonSprawl is not set."); + } + + await using var conn = new NpgsqlConnection(cs); + await conn.OpenAsync(); + await using var cmd = new NpgsqlCommand( + """ + INSERT INTO player_position (player_id, pos_x, pos_y, pos_z, sequence) + VALUES (@pid, 0, 0, 0, 0) + ON CONFLICT (player_id) DO NOTHING; + """, + conn); + cmd.Parameters.AddWithValue("pid", playerId); + await cmd.ExecuteNonQueryAsync(); + } + private async Task ResetContractTablesAsync() { var cs = Environment.GetEnvironmentVariable("ConnectionStrings__NeonSprawl"); diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs index 9acbaf2..ac1783c 100644 --- a/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs @@ -93,6 +93,20 @@ public sealed class InMemoryContractOutcomeStoreTests Assert.Empty(store.GetOutcomesForInstance(InstanceId)); } + [Fact] + public void TryAppend_ShouldReturnFalse_WhenPlayerIdDoesNotMatchInstanceOwner() + { + // Arrange + var (instanceStore, store) = CreateStores(); + SeedActiveInstance(instanceStore); + var row = CreatePrototypeOutcome() with { PlayerId = "other-player-xyz" }; + // Act + var appended = store.TryAppend(row); + // Assert + Assert.False(appended); + Assert.Empty(store.GetOutcomesForInstance(InstanceId)); + } + private static (InMemoryContractInstanceStore InstanceStore, InMemoryContractOutcomeStore OutcomeStore) CreateStores() { var options = Options.Create(new GamePositionOptions { DevPlayerId = PlayerId }); diff --git a/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeBootstrap.cs b/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeBootstrap.cs index 3e56365..ef6f253 100644 --- a/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeBootstrap.cs +++ b/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeBootstrap.cs @@ -13,6 +13,8 @@ public static class PostgresContractOutcomeBootstrap public static void EnsureSchema(Npgsql.NpgsqlDataSource dataSource) { + PostgresContractInstanceBootstrap.EnsureSchema(dataSource); + if (Volatile.Read(ref _schemaReady) != 0) { return; diff --git a/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs b/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs index fcd694d..f9bab91 100644 --- a/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs +++ b/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs @@ -29,7 +29,10 @@ public sealed class PostgresContractOutcomeStore(Npgsql.NpgsqlDataSource dataSou SELECT @id, @instance_id, @pid, @key, @items::jsonb, @skill_xp::jsonb, @rep::jsonb, @recorded WHERE NOT EXISTS ( SELECT 1 FROM contract_outcome - WHERE id = @id OR idempotency_key = @key); + WHERE id = @id OR idempotency_key = @key) + AND EXISTS ( + SELECT 1 FROM contract_instance + WHERE contract_instance_id = @instance_id AND player_id = @pid); """, conn); cmd.Parameters.AddWithValue("id", normalized.Id); @@ -44,7 +47,9 @@ public sealed class PostgresContractOutcomeStore(Npgsql.NpgsqlDataSource dataSou { return cmd.ExecuteNonQuery() == 1; } - catch (Npgsql.PostgresException ex) when (ex.SqlState == Npgsql.PostgresErrorCodes.ForeignKeyViolation) + catch (Npgsql.PostgresException ex) when ( + ex.SqlState == Npgsql.PostgresErrorCodes.ForeignKeyViolation || + ex.SqlState == Npgsql.PostgresErrorCodes.UniqueViolation) { return false; } diff --git a/server/README.md b/server/README.md index dc42b43..ce67670 100644 --- a/server/README.md +++ b/server/README.md @@ -292,7 +292,7 @@ Append-only contract completion audit rows live in **`IContractOutcomeStore`**. **Store interface methods:** -- **`TryAppend`** — first row id or idempotency key returns `true`; duplicates return `false`. Missing **`contract_instance`** row returns `false` (Postgres FK; in-memory checks **`IContractInstanceStore.TryGet`**) — orchestrators (NEO-149) must create the instance before appending an outcome. +- **`TryAppend`** — first row id or idempotency key returns `true`; duplicates return `false`. Missing **`contract_instance`** row, or a **`player_id`** that does not match the instance owner, returns `false` (Postgres SQL guard + FK/unique catch; in-memory checks **`IContractInstanceStore.TryGet`**) — orchestrators (NEO-149) must create the instance before appending an outcome. - **`TryGetByIdempotencyKey`** — read one outcome for delivery replay / audit tests. - **`GetOutcomesForInstance`** — list outcomes for one contract instance id (ordered by recorded time). - **`TryClearForInstance`** — dev fixture only. From 6c4221072a90da6b8a2c9cb7be1d9bd28a724211 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 24 Jun 2026 21:05:56 -0400 Subject: [PATCH 14/18] NEO-146: fix in-memory create/complete lock ordering Nest player and instance locks in TryCreateActive, TryMarkComplete, and TryClearInstance so concurrent create and complete cannot leave a stale active index or mismatched snapshot. --- docs/plans/NEO-146-implementation-plan.md | 2 +- .../InMemoryContractInstanceStoreTests.cs | 38 ++++++ .../InMemoryContractInstanceStore.cs | 117 ++++++++++-------- 3 files changed, 102 insertions(+), 55 deletions(-) diff --git a/docs/plans/NEO-146-implementation-plan.md b/docs/plans/NEO-146-implementation-plan.md index 78be83c..dcdd51b 100644 --- a/docs/plans/NEO-146-implementation-plan.md +++ b/docs/plans/NEO-146-implementation-plan.md @@ -68,7 +68,7 @@ Durable per-player **`ContractInstance`** rows (active/completed) and append-onl - **Types:** `ContractInstanceStatus`, `ContractInstanceState`, `ContractInstanceIds`, `ContractInstanceReasonCodes`, `ContractOutcomeRow`, `ContractOutcomeIds`. - **Stores:** `IContractInstanceStore` / `IContractOutcomeStore`; in-memory + Postgres (`V011`, `V012`) when connection string set; partial unique index + app-level one-active enforcement. - **DI:** `AddContractInstanceStores` in `Program.cs`; in-memory override in `InMemoryWebApplicationFactory`. -- **Tests:** `InMemoryContractInstanceStoreTests`, `InMemoryContractOutcomeStoreTests`, Postgres persistence integration tests; host DI smoke (`871` tests green). +- **Tests:** `InMemoryContractInstanceStoreTests`, `InMemoryContractOutcomeStoreTests`, Postgres persistence integration tests; host DI smoke (`872` tests green). - **In-memory outcome guard:** `InMemoryContractOutcomeStore.TryAppend` denies when **`IContractInstanceStore.TryGet`** is false (Postgres FK + player/instance SQL guard parity). - **Docs:** `server/README.md` contract instance + outcome store sections. - **Bruno:** `bruno/neon-sprawl-server/contract-stores/` health smoke (startup-only; no contract HTTP in E7M4-03). diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs index 8756c34..2f53aba 100644 --- a/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs @@ -227,6 +227,44 @@ public sealed class InMemoryContractInstanceStoreTests Assert.False(created); } + [Fact] + public async Task TryCreateActiveAndTryMarkComplete_ShouldKeepActiveIndexConsistent_WhenRacingOnSameInstanceId() + { + // Arrange + var store = CreateStore(); + // Act + await Task.WhenAll( + Enumerable.Range(0, 32).Select(i => Task.Run(() => + { + if (i % 2 == 0) + { + _ = store.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _); + return; + } + + _ = store.TryMarkComplete(PlayerId, InstanceId, CompletedAt, out _); + }))); + + // Assert + if (store.TryGetActiveForPlayer(PlayerId, out var active)) + { + Assert.True(store.TryGet(PlayerId, active.ContractInstanceId, out var row)); + Assert.Equal(ContractInstanceStatus.Active, row.Status); + } + + if (store.TryGet(PlayerId, InstanceId, out var final) && + final.Status == ContractInstanceStatus.Completed) + { + Assert.False(store.TryGetActiveForPlayer(PlayerId, out _)); + } + } + [Fact] public async Task Host_ShouldResolveContractInstanceStoresFromDi() { diff --git a/server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs b/server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs index 8b38e8b..acd30b6 100644 --- a/server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs +++ b/server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs @@ -153,36 +153,39 @@ public sealed class InMemoryContractInstanceStore(IOptions lock (instanceLocks.GetOrAdd(PlayerLockKey(player), _ => new object())) { - if (activeInstanceByPlayer.TryGetValue(player, out var existingActiveId)) + lock (instanceLocks.GetOrAdd(ContractInstanceIds.MakeInstanceKey(player, instanceId), _ => new object())) { - if (byInstanceId.TryGetValue(existingActiveId, out var existingActive) && - existingActive.Status == ContractInstanceStatus.Active) + if (activeInstanceByPlayer.TryGetValue(player, out var existingActiveId)) { - snapshot = existingActive.ToSnapshot(); + if (byInstanceId.TryGetValue(existingActiveId, out var existingActive) && + existingActive.Status == ContractInstanceStatus.Active) + { + snapshot = existingActive.ToSnapshot(); + return false; + } + + activeInstanceByPlayer.TryRemove(player, out _); + } + + if (byInstanceId.TryGetValue(instanceId, out var existing)) + { + snapshot = existing.ToSnapshot(); return false; } - activeInstanceByPlayer.TryRemove(player, out _); + var row = new InstanceRow( + instanceId, + normalizedTemplateId, + player, + ContractInstanceStatus.Active, + normalizedSeedBucket, + issuedAt, + null); + byInstanceId[instanceId] = row; + activeInstanceByPlayer[player] = instanceId; + snapshot = row.ToSnapshot(); + return true; } - - if (byInstanceId.TryGetValue(instanceId, out var existing)) - { - snapshot = existing.ToSnapshot(); - return false; - } - - var row = new InstanceRow( - instanceId, - normalizedTemplateId, - player, - ContractInstanceStatus.Active, - normalizedSeedBucket, - issuedAt, - null); - byInstanceId[instanceId] = row; - activeInstanceByPlayer[player] = instanceId; - snapshot = row.ToSnapshot(); - return true; } } @@ -207,30 +210,33 @@ public sealed class InMemoryContractInstanceStore(IOptions return false; } - lock (instanceLocks.GetOrAdd(lockKey, _ => new object())) + lock (instanceLocks.GetOrAdd(PlayerLockKey(player), _ => new object())) { - if (!byInstanceId.TryGetValue(instanceId, out var row) || - !string.Equals(row.PlayerId, player, StringComparison.Ordinal)) + lock (instanceLocks.GetOrAdd(lockKey, _ => new object())) { - return false; - } + if (!byInstanceId.TryGetValue(instanceId, out var row) || + !string.Equals(row.PlayerId, player, StringComparison.Ordinal)) + { + return false; + } - if (row.Status == ContractInstanceStatus.Completed) - { + if (row.Status == ContractInstanceStatus.Completed) + { + snapshot = row.ToSnapshot(); + return false; + } + + if (row.Status != ContractInstanceStatus.Active) + { + snapshot = row.ToSnapshot(); + return false; + } + + row.MarkCompleted(completedAt); + activeInstanceByPlayer.TryRemove(player, out _); snapshot = row.ToSnapshot(); - return false; + return true; } - - if (row.Status != ContractInstanceStatus.Active) - { - snapshot = row.ToSnapshot(); - return false; - } - - row.MarkCompleted(completedAt); - activeInstanceByPlayer.TryRemove(player, out _); - snapshot = row.ToSnapshot(); - return true; } } @@ -250,21 +256,24 @@ public sealed class InMemoryContractInstanceStore(IOptions return false; } - lock (instanceLocks.GetOrAdd(lockKey, _ => new object())) + lock (instanceLocks.GetOrAdd(PlayerLockKey(player), _ => new object())) { - if (!byInstanceId.TryGetValue(instanceId, out var row) || - !string.Equals(row.PlayerId, player, StringComparison.Ordinal)) + lock (instanceLocks.GetOrAdd(lockKey, _ => new object())) { - return false; - } + if (!byInstanceId.TryGetValue(instanceId, out var row) || + !string.Equals(row.PlayerId, player, StringComparison.Ordinal)) + { + return false; + } - _ = byInstanceId.TryRemove(instanceId, out _); - if (row.Status == ContractInstanceStatus.Active) - { - activeInstanceByPlayer.TryRemove(player, out _); - } + _ = byInstanceId.TryRemove(instanceId, out _); + if (row.Status == ContractInstanceStatus.Active) + { + activeInstanceByPlayer.TryRemove(player, out _); + } - return true; + return true; + } } } } From 237e39c8caaf546b9356592eb3a9d02b707ee903 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 24 Jun 2026 21:11:53 -0400 Subject: [PATCH 15/18] NEO-146: simplify using declarations in outcome persistence tests Apply IDE0063 where braced scopes have no name clashes with later blocks. --- ...tractOutcomePersistenceIntegrationTests.cs | 78 +++++++++---------- 1 file changed, 36 insertions(+), 42 deletions(-) diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs index 3bd6c0a..d4f14f5 100644 --- a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs @@ -39,19 +39,17 @@ public sealed class ContractOutcomePersistenceIntegrationTests(PostgresIntegrati RecordedAt); // Act — write through first host - using (var firstScope = Factory.Services.CreateScope()) - { - var instanceStore = firstScope.ServiceProvider.GetRequiredService(); - var outcomeStore = firstScope.ServiceProvider.GetRequiredService(); - Assert.True(instanceStore.TryCreateActive( - PlayerId, - InstanceId, - TemplateId, - SeedBucket, - IssuedAt, - out _)); - Assert.True(outcomeStore.TryAppend(outcomeRow)); - } + using var firstScope = Factory.Services.CreateScope(); + var instanceStore = firstScope.ServiceProvider.GetRequiredService(); + var outcomeStore = firstScope.ServiceProvider.GetRequiredService(); + Assert.True(instanceStore.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _)); + Assert.True(outcomeStore.TryAppend(outcomeRow)); ContractOutcomeRow readBack; await using var secondFactory = new PostgresWebApplicationFactory(); @@ -145,24 +143,22 @@ public sealed class ContractOutcomePersistenceIntegrationTests(PostgresIntegrati [], [], RecordedAt); - using (var scope = Factory.Services.CreateScope()) - { - var instanceStore = scope.ServiceProvider.GetRequiredService(); - var outcomeStore = scope.ServiceProvider.GetRequiredService(); - Assert.True(instanceStore.TryCreateActive( - PlayerId, - InstanceId, - TemplateId, - SeedBucket, - IssuedAt, - out _)); - await SeedPostgresPlayerAsync(otherPlayerId); - // Act - var appended = outcomeStore.TryAppend(outcomeRow); - // Assert - Assert.False(appended); - Assert.Empty(outcomeStore.GetOutcomesForInstance(InstanceId)); - } + using var scope = Factory.Services.CreateScope(); + var instanceStore = scope.ServiceProvider.GetRequiredService(); + var outcomeStore = scope.ServiceProvider.GetRequiredService(); + Assert.True(instanceStore.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _)); + await SeedPostgresPlayerAsync(otherPlayerId); + // Act + var appended = outcomeStore.TryAppend(outcomeRow); + // Assert + Assert.False(appended); + Assert.Empty(outcomeStore.GetOutcomesForInstance(InstanceId)); } [RequirePostgresFact] @@ -180,17 +176,15 @@ public sealed class ContractOutcomePersistenceIntegrationTests(PostgresIntegrati [], [], RecordedAt); - using (var scope = Factory.Services.CreateScope()) - { - var instanceStore = scope.ServiceProvider.GetRequiredService(); - Assert.True(instanceStore.TryCreateActive( - PlayerId, - InstanceId, - TemplateId, - SeedBucket, - IssuedAt, - out _)); - } + using var seedScope = Factory.Services.CreateScope(); + var instanceStore = seedScope.ServiceProvider.GetRequiredService(); + Assert.True(instanceStore.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _)); // Act var results = await Task.WhenAll( From 93bd45d49a5782d995e3fb7464609f9c0c693d50 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 24 Jun 2026 21:13:42 -0400 Subject: [PATCH 16/18] NEO-146: fix in-memory outcome idempotency race and clear parity Lock idempotency keys alongside row ids in TryAppend so concurrent duplicate keys cannot both succeed; return true from TryClearInstance when the row is absent to match Postgres idempotent DELETE behavior. --- .../InMemoryContractInstanceStoreTests.cs | 12 ++++++ .../InMemoryContractOutcomeStoreTests.cs | 23 ++++++++++++ .../InMemoryContractInstanceStore.cs | 2 +- .../Contracts/InMemoryContractOutcomeStore.cs | 37 +++++++++++++++++-- 4 files changed, 70 insertions(+), 4 deletions(-) diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs index 2f53aba..b3a3919 100644 --- a/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs @@ -265,6 +265,18 @@ public sealed class InMemoryContractInstanceStoreTests } } + [Fact] + public void TryClearInstance_ShouldReturnTrue_WhenRowMissing() + { + // Arrange + var store = CreateStore(); + // Act + var cleared = store.TryClearInstance(PlayerId, InstanceId); + // Assert + Assert.True(cleared); + Assert.False(store.TryGet(PlayerId, InstanceId, out _)); + } + [Fact] public async Task Host_ShouldResolveContractInstanceStoresFromDi() { diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs index ac1783c..2aa283d 100644 --- a/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs @@ -107,6 +107,29 @@ public sealed class InMemoryContractOutcomeStoreTests Assert.Empty(store.GetOutcomesForInstance(InstanceId)); } + [Fact] + public async Task TryAppend_ShouldAllowOnlyOneSuccess_WhenConcurrentDuplicateIdempotencyKeysRace() + { + // Arrange + var (instanceStore, store) = CreateStores(); + SeedActiveInstance(instanceStore); + var idempotencyKey = ContractOutcomeIds.MakeIdempotencyKey(PlayerId, InstanceId); + var baseRow = CreatePrototypeOutcome(); + // Act + var results = await Task.WhenAll( + Enumerable.Range(0, 8).Select(i => Task.Run(() => + { + var row = baseRow with { Id = $"outcome-row-race-{i:D3}" }; + Assert.Equal(idempotencyKey, row.IdempotencyKey); + return store.TryAppend(row); + }))); + + // Assert + Assert.Equal(1, results.Count(static r => r)); + Assert.Equal(7, results.Count(static r => !r)); + Assert.Single(store.GetOutcomesForInstance(InstanceId)); + } + private static (InMemoryContractInstanceStore InstanceStore, InMemoryContractOutcomeStore OutcomeStore) CreateStores() { var options = Options.Create(new GamePositionOptions { DevPlayerId = PlayerId }); diff --git a/server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs b/server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs index acd30b6..5c7d208 100644 --- a/server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs +++ b/server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs @@ -263,7 +263,7 @@ public sealed class InMemoryContractInstanceStore(IOptions if (!byInstanceId.TryGetValue(instanceId, out var row) || !string.Equals(row.PlayerId, player, StringComparison.Ordinal)) { - return false; + return true; } _ = byInstanceId.TryRemove(instanceId, out _); diff --git a/server/NeonSprawl.Server/Game/Contracts/InMemoryContractOutcomeStore.cs b/server/NeonSprawl.Server/Game/Contracts/InMemoryContractOutcomeStore.cs index 7340c06..87080aa 100644 --- a/server/NeonSprawl.Server/Game/Contracts/InMemoryContractOutcomeStore.cs +++ b/server/NeonSprawl.Server/Game/Contracts/InMemoryContractOutcomeStore.cs @@ -25,17 +25,47 @@ public sealed class InMemoryContractOutcomeStore(IContractInstanceStore instance return false; } - lock (idLocks.GetOrAdd(normalized.Id, _ => new object())) + var appended = false; + LockRowAndIdempotencyKey(normalized.Id, normalized.IdempotencyKey, () => { if (rowsById.ContainsKey(normalized.Id) || rowIdByIdempotencyKey.ContainsKey(normalized.IdempotencyKey)) { - return false; + return; } rowsById[normalized.Id] = normalized; rowIdByIdempotencyKey[normalized.IdempotencyKey] = normalized.Id; - return true; + appended = true; + }); + + return appended; + } + + private static string IdempotencyLockKey(string idempotencyKey) => $"idem\0{idempotencyKey}"; + + private void LockRowAndIdempotencyKey(string rowId, string idempotencyKey, Action action) + { + var idemLockKey = IdempotencyLockKey(idempotencyKey); + if (string.CompareOrdinal(rowId, idemLockKey) <= 0) + { + lock (idLocks.GetOrAdd(rowId, _ => new object())) + { + lock (idLocks.GetOrAdd(idemLockKey, _ => new object())) + { + action(); + } + } + + return; + } + + lock (idLocks.GetOrAdd(idemLockKey, _ => new object())) + { + lock (idLocks.GetOrAdd(rowId, _ => new object())) + { + action(); + } } } @@ -99,6 +129,7 @@ public sealed class InMemoryContractOutcomeStore(IContractInstanceStore instance if (rowsById.TryRemove(id, out var removed)) { rowIdByIdempotencyKey.TryRemove(removed.IdempotencyKey, out _); + idLocks.TryRemove(IdempotencyLockKey(removed.IdempotencyKey), out _); } idLocks.TryRemove(id, out _); From a64f7100a2ed6a1355159ea6732051b77f48be48 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 24 Jun 2026 21:13:44 -0400 Subject: [PATCH 17/18] NEO-146: update plan reconciliation test count to 874 --- docs/plans/NEO-146-implementation-plan.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/plans/NEO-146-implementation-plan.md b/docs/plans/NEO-146-implementation-plan.md index dcdd51b..3d6bc93 100644 --- a/docs/plans/NEO-146-implementation-plan.md +++ b/docs/plans/NEO-146-implementation-plan.md @@ -68,7 +68,7 @@ Durable per-player **`ContractInstance`** rows (active/completed) and append-onl - **Types:** `ContractInstanceStatus`, `ContractInstanceState`, `ContractInstanceIds`, `ContractInstanceReasonCodes`, `ContractOutcomeRow`, `ContractOutcomeIds`. - **Stores:** `IContractInstanceStore` / `IContractOutcomeStore`; in-memory + Postgres (`V011`, `V012`) when connection string set; partial unique index + app-level one-active enforcement. - **DI:** `AddContractInstanceStores` in `Program.cs`; in-memory override in `InMemoryWebApplicationFactory`. -- **Tests:** `InMemoryContractInstanceStoreTests`, `InMemoryContractOutcomeStoreTests`, Postgres persistence integration tests; host DI smoke (`872` tests green). +- **Tests:** `InMemoryContractInstanceStoreTests`, `InMemoryContractOutcomeStoreTests`, Postgres persistence integration tests; host DI smoke (`874` tests green). - **In-memory outcome guard:** `InMemoryContractOutcomeStore.TryAppend` denies when **`IContractInstanceStore.TryGet`** is false (Postgres FK + player/instance SQL guard parity). - **Docs:** `server/README.md` contract instance + outcome store sections. - **Bruno:** `bruno/neon-sprawl-server/contract-stores/` health smoke (startup-only; no contract HTTP in E7M4-03). From 1cf80a4e4e4d036f8946fb1d9e6b191d74f54616 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 24 Jun 2026 21:21:02 -0400 Subject: [PATCH 18/18] NEO-146: fix stale active index race in TryGetActiveForPlayer Read the active index under the player lock and remove stale entries only when the indexed instance id still matches, preventing concurrent get/complete/reissue from dropping a newer active pointer. --- docs/plans/NEO-146-implementation-plan.md | 2 +- .../InMemoryContractInstanceStoreTests.cs | 51 +++++++++++++++++++ .../InMemoryContractInstanceStore.cs | 18 +++---- 3 files changed, 61 insertions(+), 10 deletions(-) diff --git a/docs/plans/NEO-146-implementation-plan.md b/docs/plans/NEO-146-implementation-plan.md index 3d6bc93..f113382 100644 --- a/docs/plans/NEO-146-implementation-plan.md +++ b/docs/plans/NEO-146-implementation-plan.md @@ -68,7 +68,7 @@ Durable per-player **`ContractInstance`** rows (active/completed) and append-onl - **Types:** `ContractInstanceStatus`, `ContractInstanceState`, `ContractInstanceIds`, `ContractInstanceReasonCodes`, `ContractOutcomeRow`, `ContractOutcomeIds`. - **Stores:** `IContractInstanceStore` / `IContractOutcomeStore`; in-memory + Postgres (`V011`, `V012`) when connection string set; partial unique index + app-level one-active enforcement. - **DI:** `AddContractInstanceStores` in `Program.cs`; in-memory override in `InMemoryWebApplicationFactory`. -- **Tests:** `InMemoryContractInstanceStoreTests`, `InMemoryContractOutcomeStoreTests`, Postgres persistence integration tests; host DI smoke (`874` tests green). +- **Tests:** `InMemoryContractInstanceStoreTests`, `InMemoryContractOutcomeStoreTests`, Postgres persistence integration tests; host DI smoke (`875` tests green). - **In-memory outcome guard:** `InMemoryContractOutcomeStore.TryAppend` denies when **`IContractInstanceStore.TryGet`** is false (Postgres FK + player/instance SQL guard parity). - **Docs:** `server/README.md` contract instance + outcome store sections. - **Bruno:** `bruno/neon-sprawl-server/contract-stores/` health smoke (startup-only; no contract HTTP in E7M4-03). diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs index b3a3919..e8215bf 100644 --- a/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractInstanceStoreTests.cs @@ -265,6 +265,57 @@ public sealed class InMemoryContractInstanceStoreTests } } + [Fact] + public async Task TryGetActiveForPlayer_ShouldKeepActiveIndexConsistent_WhenRacingWithCompleteAndReissue() + { + // Arrange + var store = CreateStore(); + Assert.True(store.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _)); + // Act + await Task.WhenAll( + Enumerable.Range(0, 32).Select(i => Task.Run(() => + { + if (i % 3 == 0) + { + _ = store.TryGetActiveForPlayer(PlayerId, out _); + return; + } + + if (i % 3 == 1) + { + _ = store.TryMarkComplete(PlayerId, InstanceId, CompletedAt, out _); + return; + } + + _ = store.TryCreateActive( + PlayerId, + SecondInstanceId, + TemplateId, + SeedBucket, + IssuedAt.AddDays(1), + out _); + }))); + + // Assert + if (store.TryGetActiveForPlayer(PlayerId, out var active)) + { + Assert.True(store.TryGet(PlayerId, active.ContractInstanceId, out var row)); + Assert.Equal(ContractInstanceStatus.Active, row.Status); + } + + Assert.False( + store.TryGet(PlayerId, InstanceId, out var first) && + store.TryGet(PlayerId, SecondInstanceId, out var second) && + first.Status == ContractInstanceStatus.Active && + second.Status == ContractInstanceStatus.Active); + } + [Fact] public void TryClearInstance_ShouldReturnTrue_WhenRowMissing() { diff --git a/server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs b/server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs index 5c7d208..36591ca 100644 --- a/server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs +++ b/server/NeonSprawl.Server/Game/Contracts/InMemoryContractInstanceStore.cs @@ -108,18 +108,18 @@ public sealed class InMemoryContractInstanceStore(IOptions return false; } - if (!activeInstanceByPlayer.TryGetValue(player, out var instanceId)) - { - return false; - } - lock (instanceLocks.GetOrAdd(PlayerLockKey(player), _ => new object())) { + if (!activeInstanceByPlayer.TryGetValue(player, out var instanceId)) + { + return false; + } + if (!byInstanceId.TryGetValue(instanceId, out var row) || row.Status != ContractInstanceStatus.Active || !string.Equals(row.PlayerId, player, StringComparison.Ordinal)) { - activeInstanceByPlayer.TryRemove(player, out _); + activeInstanceByPlayer.TryRemove(new KeyValuePair(player, instanceId)); return false; } @@ -164,7 +164,7 @@ public sealed class InMemoryContractInstanceStore(IOptions return false; } - activeInstanceByPlayer.TryRemove(player, out _); + activeInstanceByPlayer.TryRemove(new KeyValuePair(player, existingActiveId)); } if (byInstanceId.TryGetValue(instanceId, out var existing)) @@ -233,7 +233,7 @@ public sealed class InMemoryContractInstanceStore(IOptions } row.MarkCompleted(completedAt); - activeInstanceByPlayer.TryRemove(player, out _); + activeInstanceByPlayer.TryRemove(new KeyValuePair(player, instanceId)); snapshot = row.ToSnapshot(); return true; } @@ -269,7 +269,7 @@ public sealed class InMemoryContractInstanceStore(IOptions _ = byInstanceId.TryRemove(instanceId, out _); if (row.Status == ContractInstanceStatus.Active) { - activeInstanceByPlayer.TryRemove(player, out _); + activeInstanceByPlayer.TryRemove(new KeyValuePair(player, instanceId)); } return true;