NEO-146: add E7M4-03 contract instance and outcome stores kickoff plan

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<RewardItemGrantApplied> GrantedItems`
+- `IReadOnlyList<RewardSkillXpGrantApplied> GrantedSkillXp`
+- `IReadOnlyList<RewardReputationGrantApplied> 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<ContractOutcomeRow> 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**.