From 35fa4ad1927b3a04f3739eb845eddffb67aa8a7d Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 7 Jun 2026 17:33:07 -0400 Subject: [PATCH] =?UTF-8?q?NEO-126:=20address=20code=20review=20=E2=80=94?= =?UTF-8?q?=20module=20anchor=20and=20casing=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add E7.M2 implementation anchor for the delivery store and assert mixed-case TryRecord replays are rejected at the store layer. --- .../decomposition/modules/E7_M2_RewardAndUnlockRouter.md | 2 ++ docs/plans/NEO-126-implementation-plan.md | 2 +- docs/reviews/2026-06-07-NEO-126.md | 9 +++++---- .../Game/Rewards/InMemoryRewardDeliveryStoreTests.cs | 7 +++++++ 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md b/docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md index 262265b..06e8032 100644 --- a/docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md +++ b/docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md @@ -67,3 +67,5 @@ Epic 7 **Slice 2** — `reward_delivery`, `unlock_granted`; no double-claim on r **NEO-43:** When applying scripted **skill XP** from a mission/quest reward bundle, call **`MissionRewardSkillXpGrant.GrantFromMissionReward`** (`server/NeonSprawl.Server/Game/Skills/`) — fixed **`sourceKind: mission_reward`**, **`skillId`** and **`amount`** from reward data — [NEO-43 implementation plan](../../plans/NEO-43-implementation-plan.md). **NEO-107 (E5.M3 producer hook):** On encounter completion grant commit, **`EncounterCompletionOperations.TryCompleteAndGrant`** records **`EncounterCompleteEvent`** in **`IEncounterCompleteEventStore`** (idempotency key **`{playerId}:{encounterId}`**). Future E7.M2 **`QuestRewardBundle`** router consumes this record for **`reward_delivery`** quest credit beyond prototype **`contract_handoff_token`** item loot — comment-only hook stub; no runtime router yet. See [NEO-107 implementation plan](../../plans/NEO-107-implementation-plan.md); [server README — Encounter complete event (NEO-107)](../../../server/README.md#encounter-complete-event-record-neo-107). + +**NEO-126 (E7M2-03 delivery store):** Quest completion reward delivery uses **`IRewardDeliveryStore`** + **`InMemoryRewardDeliveryStore`** in `server/NeonSprawl.Server/Game/Rewards/` — idempotent **`RewardDeliveryEvent`** per player+quest (idempotency key **`{playerId}:quest_complete:{questId}`**; grant snapshots for item + skill XP rows). Router apply and quest-state wiring land in NEO-127/NEO-128; HTTP **`completionRewardSummary`** in NEO-129. See [NEO-126 implementation plan](../../plans/NEO-126-implementation-plan.md); [server README — Reward delivery store (NEO-126)](../../../server/README.md#reward-delivery-store-neo-126). diff --git a/docs/plans/NEO-126-implementation-plan.md b/docs/plans/NEO-126-implementation-plan.md index 80c24f6..eab7e30 100644 --- a/docs/plans/NEO-126-implementation-plan.md +++ b/docs/plans/NEO-126-implementation-plan.md @@ -58,7 +58,7 @@ - **Types:** `RewardDeliveryEvent`, `RewardItemGrantApplied`, `RewardSkillXpGrantApplied`, `RewardDeliveryIds` in `Game/Rewards/`. - **Store:** `IRewardDeliveryStore` + `InMemoryRewardDeliveryStore` (`TryRecord`, `TryGet`, dev `TryClear`). - **DI:** `AddRewardDeliveryStore()` in `Program.cs` — in-memory singleton only (Postgres deferred per kickoff). -- **Tests:** 5 AAA tests in `InMemoryRewardDeliveryStoreTests` (idempotency, fail-closed, grant summary, host DI resolve). +- **Tests:** 5 AAA tests in `InMemoryRewardDeliveryStoreTests` (idempotency incl. mixed-case replay, fail-closed, grant summary, host DI resolve). - **Docs:** `server/README.md` reward delivery store section; E7M2-03 backlog AC checked; alignment register updated. - **Bruno:** `bruno/neon-sprawl-server/quest-progress/Health after reward delivery store load.bru` (host boot smoke; pre-commit gate for `Program.cs`). diff --git a/docs/reviews/2026-06-07-NEO-126.md b/docs/reviews/2026-06-07-NEO-126.md index 4f67fe6..231753a 100644 --- a/docs/reviews/2026-06-07-NEO-126.md +++ b/docs/reviews/2026-06-07-NEO-126.md @@ -2,7 +2,8 @@ **Date:** 2026-06-07 **Scope:** Branch `NEO-126-e7m2-reward-delivery-store` vs `origin/main` — commits `b20d9c1` … `2a8fe31` -**Base:** `origin/main` +**Base:** `origin/main` +**Follow-up:** Suggestions below are **done** (strikethrough + **Done.**). ## Verdict @@ -18,7 +19,7 @@ NEO-126 adds **`IRewardDeliveryStore`** and **`InMemoryRewardDeliveryStore`** un |------|--------| | `docs/plans/NEO-126-implementation-plan.md` | **Matches** — kickoff (in-memory only), acceptance criteria checked, reconciliation accurate. | | `docs/plans/E7M2-prototype-backlog.md` (E7M2-03) | **Matches** — AC checkboxes complete; infrastructure-only client counterpart noted. | -| `docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md` | **Partially matches** — freeze idempotency key and **`RewardDeliveryEvent`** contract align; no NEO-126 implementation anchor yet (NEO-125 added a server-load paragraph; NEO-107 has encounter-store anchor). | +| `docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md` | **Matches** — NEO-126 implementation anchor added (review follow-up). | | `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E7.M2 row notes E7M2-03 delivery store; row correctly stays **Planned** until NEO-132 capstone. | | `docs/decomposition/modules/module_dependency_register.md` | **Matches** — no register change required for this slice. | | `server/README.md` | **Matches** — reward delivery store section documents interface, event fields, idempotency key, NEO-127/NEO-129 pointers. | @@ -32,9 +33,9 @@ None. ## Suggestions -1. **E7.M2 module anchor** — Add a short **Implementation anchor (server)** bullet for NEO-126 in `docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md` (parity with the NEO-125 server-load and NEO-107 encounter-store paragraphs) so decomposition readers do not rely only on the alignment register and README. +1. ~~**E7.M2 module anchor** — Add a short **Implementation anchor (server)** bullet for NEO-126 in `docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md` (parity with the NEO-125 server-load and NEO-107 encounter-store paragraphs) so decomposition readers do not rely only on the alignment register and README.~~ **Done.** NEO-126 paragraph under **Implementation anchor (server)** in `E7_M2_RewardAndUnlockRouter.md`. -2. **Case-normalization idempotency test** — `MakeStoreKey` lowercases ids, but no test proves a second `TryRecord` with different casing of the same player/quest is rejected. A one-liner extension of `TryRecord_ShouldBeIdempotent_AtStoreLayer` (e.g. replay with `PlayerId = "DEV-LOCAL-1"`) would lock that behavior for NEO-127 replays. +2. ~~**Case-normalization idempotency test** — `MakeStoreKey` lowercases ids, but no test proves a second `TryRecord` with different casing of the same player/quest is rejected. A one-liner extension of `TryRecord_ShouldBeIdempotent_AtStoreLayer` (e.g. replay with `PlayerId = "DEV-LOCAL-1"`) would lock that behavior for NEO-127 replays.~~ **Done.** Third replay with mixed-case ids in `InMemoryRewardDeliveryStoreTests.TryRecord_ShouldBeIdempotent_AtStoreLayer`. ## Nits diff --git a/server/NeonSprawl.Server.Tests/Game/Rewards/InMemoryRewardDeliveryStoreTests.cs b/server/NeonSprawl.Server.Tests/Game/Rewards/InMemoryRewardDeliveryStoreTests.cs index e6f4055..8cf000a 100644 --- a/server/NeonSprawl.Server.Tests/Game/Rewards/InMemoryRewardDeliveryStoreTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Rewards/InMemoryRewardDeliveryStoreTests.cs @@ -20,9 +20,16 @@ public sealed class InMemoryRewardDeliveryStoreTests // Act var first = store.TryRecord(deliveryEvent); var second = store.TryRecord(deliveryEvent with { DeliveredAt = DeliveredAt.AddHours(1) }); + var third = store.TryRecord( + deliveryEvent with + { + PlayerId = "DEV-LOCAL-1", + QuestId = "PROTOTYPE_QUEST_OPERATOR_CHAIN", + }); // Assert Assert.True(first); Assert.False(second); + Assert.False(third); Assert.True(store.TryGet(PlayerId, QuestId, out var stored)); Assert.Equal(deliveryEvent.IdempotencyKey, stored.IdempotencyKey); Assert.Equal(DeliveredAt, stored.DeliveredAt);