NEO-126: add E7M2-03 reward delivery store implementation plan

Kickoff plan for IRewardDeliveryStore — in-memory idempotent
RewardDeliveryEvent records keyed by quest completion.
pull/165/head
VinPropane 2026-06-07 17:30:14 -04:00
parent 0372991d87
commit b20d9c18b5
1 changed files with 159 additions and 0 deletions

View File

@ -0,0 +1,159 @@
# NEO-126 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEO-126 |
| **Title** | E7M2-03: Reward delivery store (`IRewardDeliveryStore`) |
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-126/e7m2-03-reward-delivery-store-irewarddeliverystore |
| **Module** | [E7.M2 — RewardAndUnlockRouter](../decomposition/modules/E7_M2_RewardAndUnlockRouter.md) · Epic 7 Slice 2 · backlog **E7M2-03** |
| **Branch** | `NEO-126-e7m2-reward-delivery-store` |
| **Precursor** | [NEO-124](https://linear.app/neon-sprawl/issue/NEO-124) **`Done`** — bundle schemas + catalog rows · [NEO-125](https://linear.app/neon-sprawl/issue/NEO-125) **`Done`** — server catalog load + E7M2 gates |
| **Pattern** | [NEO-107](NEO-107-implementation-plan.md) — `IEncounterCompleteEventStore` / `InMemoryEncounterCompleteEventStore` idempotent `TryRecord` + `TryGet` |
| **Blocks** | [NEO-127](https://linear.app/neon-sprawl/issue/NEO-127) (`RewardRouterOperations` apply path) |
| **Client counterpart** | none — infrastructure-only ([E7M2-prototype-backlog](E7M2-prototype-backlog.md#e7m2-03--reward-delivery-store-irewarddeliverystore)) |
## Kickoff clarifications
| Topic | Question | Agent recommendation | Answer |
|--------|----------|----------------------|--------|
| **Postgres persistence** | Implement NEO-29-style Postgres split now or in-memory only? | **In-memory only** — mirror `IEncounterCompleteEventStore`; NEO-126 AC is store idempotency + readable summary, not cross-restart durability; Postgres deferred until a story needs persisted delivery rows. | **Adopted** — in-memory only |
**Additional defaults (no kickoff question — settled by backlog / precedent):**
- **Idempotency key:** `{normalizedPlayerId}:quest_complete:{normalizedQuestId}` per [E7M2 kickoff decisions](E7M2-prototype-backlog.md#kickoff-decisions-decomposition-defaults) and [E7.M2 module](../decomposition/modules/E7_M2_RewardAndUnlockRouter.md#prototype-slice-2-freeze-e7m2-01).
- **Namespace:** `Game/Rewards/` — delivery is cross-cutting (quest completion v1; encounter consumer later); keeps `Game/Quests/` for state machine only.
- **Grant summary rows:** dedicated `RewardItemGrantApplied` + `RewardSkillXpGrantApplied` on the event (do not reuse `EncounterGrantApplied` — quest bundles include skill XP lines).
- **Store key:** composite `playerId\0questId` via `RewardDeliveryIds.MakeStoreKey` for `TryGet(playerId, questId)`; event carries explicit **`IdempotencyKey`** string for router audit.
- **Dev `TryClear`:** optional on `InMemoryRewardDeliveryStore` only (Bruno / test reset parity with `InMemoryEncounterCompleteEventStore`).
## Goal, scope, and out-of-scope
**Goal:** Durable idempotency record for **`RewardDeliveryEvent`** per player + quest completion.
**In scope (from Linear + [E7M2-03](E7M2-prototype-backlog.md#e7m2-03--reward-delivery-store-irewarddeliverystore)):**
- `IRewardDeliveryStore`, in-memory implementation.
- Idempotency key **`{playerId}:quest_complete:{questId}`**; store granted item + skill XP summary for audit/read projection.
- Unit tests (AAA): first delivery records; replay returns existing event without double-apply (`TryRecord` → `false`).
- `server/README.md` store section.
- DI registration in `Program.cs`.
**Out of scope (from Linear):**
- Router apply logic ([NEO-127](https://linear.app/neon-sprawl/issue/NEO-127) E7M2-04).
- Quest-state wiring ([NEO-128](https://linear.app/neon-sprawl/issue/NEO-128) E7M2-05).
- HTTP `completionRewardSummary` projection ([NEO-129](https://linear.app/neon-sprawl/issue/NEO-129) E7M2-06).
- Postgres migration / `PostgresRewardDeliveryStore` (deferred per kickoff answer).
- Godot client.
## Acceptance criteria checklist
- [ ] `TryRecord` idempotent on same key.
- [ ] Store readable summary for HTTP projection.
## Technical approach
### 1. Event + grant summary types (`Game/Rewards/`)
**`RewardDeliveryEvent`** (readonly record struct):
| Field | Role |
|-------|------|
| **`PlayerId`** | Normalized player id. |
| **`QuestId`** | Normalized quest id. |
| **`DeliveredAt`** | UTC timestamp at record commit (from router/`TimeProvider` in NEO-127). |
| **`GrantedItems`** | Snapshot of applied item rows (`RewardItemGrantApplied`: `ItemId`, `Quantity`). |
| **`GrantedSkillXp`** | Snapshot of applied skill XP rows (`RewardSkillXpGrantApplied`: `SkillId`, `Amount`). |
| **`IdempotencyKey`** | Stable **`{playerId}:quest_complete:{questId}`**. |
Defensive copy on `TryRecord` (`.ToArray()` on lists) — same as `InMemoryEncounterCompleteEventStore`.
### 2. `RewardDeliveryIds`
Static helper (mirror `QuestProgressIds` / `EncounterProgressIds`):
- `NormalizePlayerId` / `NormalizeQuestId` — trim + lowercase; empty on whitespace.
- `MakeIdempotencyKey(playerId, questId)``{p}:quest_complete:{q}` or empty when invalid.
- `MakeStoreKey(playerId, questId)``{p}\0{q}` for dictionary lookup.
### 3. `IRewardDeliveryStore`
```csharp
bool TryRecord(RewardDeliveryEvent deliveryEvent);
bool TryGet(string playerId, string questId, out RewardDeliveryEvent deliveryEvent);
```
First `TryRecord` returns `true`; replays for the same player+quest return `false`. Invalid ids fail closed (`false`).
### 4. `InMemoryRewardDeliveryStore`
- `ConcurrentDictionary<string, RewardDeliveryEvent>` + per-key locks — same threading model as `InMemoryEncounterCompleteEventStore`.
- `TryClear(playerId, questId)` on concrete type only (dev fixture).
### 5. DI
**`RewardDeliveryServiceCollectionExtensions.AddRewardDeliveryStore`:**
- `services.AddSingleton<IRewardDeliveryStore, InMemoryRewardDeliveryStore>();`
- Call from **`Program.cs`** after quest catalog / quest state registration.
No connection-string branch until Postgres story.
### 6. Tests (`InMemoryRewardDeliveryStoreTests`)
Mirror `InMemoryEncounterCompleteEventStoreTests`:
- **`TryRecord_ShouldBeIdempotent_AtStoreLayer`** — first `true`, second `false`; `TryGet` returns original `DeliveredAt` + grant snapshots.
- **`TryRecord_ShouldFailClosed_WhenPlayerIdIsEmpty`**
- **`TryRecord_ShouldFailClosed_WhenQuestIdIsEmpty`**
- **`TryGet_ShouldExposeReadableGrantSummary`** — item + skill XP rows preserved in order for future HTTP mapper (NEO-129).
Optional host smoke: assert `WebApplicationFactory` resolves `IRewardDeliveryStore` as `InMemoryRewardDeliveryStore` (same pattern as `EncounterDefinitionRegistryTests`).
### 7. Docs
- **`server/README.md`** — new **Reward delivery store (NEO-126)** section: interface, event fields, idempotency key, in-memory prototype policy, pointer to NEO-127 router consumer.
- **`documentation_and_implementation_alignment.md`** — E7.M2 row note: delivery store landed (NEO-126).
## Files to add
| Path | Purpose |
|------|---------|
| `server/NeonSprawl.Server/Game/Rewards/RewardItemGrantApplied.cs` | Item grant snapshot on a delivery event. |
| `server/NeonSprawl.Server/Game/Rewards/RewardSkillXpGrantApplied.cs` | Skill XP grant snapshot on a delivery event. |
| `server/NeonSprawl.Server/Game/Rewards/RewardDeliveryEvent.cs` | Idempotent delivery payload (quest completion v1). |
| `server/NeonSprawl.Server/Game/Rewards/RewardDeliveryIds.cs` | Id normalization, store key, idempotency key helper. |
| `server/NeonSprawl.Server/Game/Rewards/IRewardDeliveryStore.cs` | Store contract (`TryRecord`, `TryGet`). |
| `server/NeonSprawl.Server/Game/Rewards/InMemoryRewardDeliveryStore.cs` | Thread-safe in-memory implementation + dev `TryClear`. |
| `server/NeonSprawl.Server/Game/Rewards/RewardDeliveryServiceCollectionExtensions.cs` | DI registration. |
| `server/NeonSprawl.Server.Tests/Game/Rewards/InMemoryRewardDeliveryStoreTests.cs` | AAA idempotency + fail-closed + summary tests. |
| `docs/plans/NEO-126-implementation-plan.md` | This plan. |
## Files to modify
| Path | Rationale |
|------|-----------|
| `server/NeonSprawl.Server/Program.cs` | Register `AddRewardDeliveryStore()`. |
| `server/README.md` | Document `IRewardDeliveryStore`, event shape, idempotency key. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E7.M2 alignment — delivery store (NEO-126). |
## Tests
| Target | Coverage |
|--------|----------|
| `server/NeonSprawl.Server.Tests/Game/Rewards/InMemoryRewardDeliveryStoreTests.cs` | Idempotent `TryRecord`; fail-closed empty ids; `TryGet` returns item + skill XP summary suitable for HTTP projection. |
| **Optional** — extend an existing host/DI test file | `IRewardDeliveryStore` resolves as `InMemoryRewardDeliveryStore` from `WebApplicationFactory`. |
| **Manual verification** | `dotnet test server/NeonSprawl.Server.Tests --filter InMemoryRewardDeliveryStoreTests`; boot dev server — no behavior change until NEO-127 wires router. |
## Open questions / risks
| Question or risk | Agent recommendation | Status |
|------------------|----------------------|--------|
| Postgres deferred vs backlog “optional migration” wording | Kickoff adopted in-memory only; add Postgres when restart durability is required (likely before production or a dedicated persistence story). | **adopted** |
| Encounter deliveries reuse same store | Out of scope — quest completion keys only in NEO-126; encounter path stays on `IEncounterCompleteEventStore` until a follow-on unifies consumers. | **adopted** |
| Empty grant arrays on event | Allow empty lists when bundle has only one grant kind; router (NEO-127) populates from apply outcome. | **adopted** |
| HTTP DTO mapping | NEO-129 maps `GrantedItems` / `GrantedSkillXp``completionRewardSummary`; no API changes in NEO-126. | **deferred** |
None beyond the above.