From 82c45941ae7ad449ac811a6f7d65f3c294e50003 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 22 Jun 2026 20:36:47 -0400 Subject: [PATCH] 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**.