diff --git a/docs/plans/NEO-127-implementation-plan.md b/docs/plans/NEO-127-implementation-plan.md new file mode 100644 index 0000000..20bacc6 --- /dev/null +++ b/docs/plans/NEO-127-implementation-plan.md @@ -0,0 +1,173 @@ +# NEO-127 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-127 | +| **Title** | E7M2-04: RewardRouterOperations (apply QuestRewardBundle) | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-127/e7m2-04-rewardrouteroperations-apply-questrewardbundle | +| **Module** | [E7.M2 — RewardAndUnlockRouter](../decomposition/modules/E7_M2_RewardAndUnlockRouter.md) · Epic 7 Slice 2 · backlog **E7M2-04** | +| **Branch** | `NEO-127-e7m2-reward-router-operations` | +| **Precursor** | [NEO-125](https://linear.app/neon-sprawl/issue/NEO-125) **`Done`** — catalog load + bundle validation · [NEO-126](https://linear.app/neon-sprawl/issue/NEO-126) **`Done`** — `IRewardDeliveryStore` + `RewardDeliveryEvent` | +| **Pattern** | [NEO-105](NEO-105-implementation-plan.md) — `EncounterCompletionOperations` simulate-all + sequential apply + compensating rollback; [NEO-107](NEO-107-implementation-plan.md) — store `TryRecord` after grants commit | +| **Blocks** | [NEO-128](https://linear.app/neon-sprawl/issue/NEO-128) (`QuestStateOperations.TryMarkComplete` wiring) | +| **Client counterpart** | none — server engine ([E7M2-prototype-backlog](E7M2-prototype-backlog.md#e7m2-04--rewardrouteroperations-apply-questrewardbundle)) | + +## Kickoff clarifications + +**No clarifications needed.** Linear AC, [E7M2-04 backlog](E7M2-prototype-backlog.md#e7m2-04--rewardrouteroperations-apply-questrewardbundle), [E7M2 kickoff decisions](E7M2-prototype-backlog.md#kickoff-decisions-decomposition-defaults), and landed NEO-105/NEO-126 code settle: + +- **Namespace `Game/Rewards/`** — co-locate router with `IRewardDeliveryStore` (NEO-126); bundle DTOs stay on `QuestDefRow` in `Game/Quests/` (NEO-125). +- **Grant order** — item grants first, skill XP second; compensating **item** rollback on inventory or skill-apply failure (NEO-105 precedent). Prototype bundles have ≤1 skill row; no XP rollback helper exists. +- **Skill XP path** — call **`SkillProgressionGrantOperations.TryApplyGrant`** with **`MissionRewardSkillXpConstants.MissionRewardSourceKind`** (not the fire-and-forget **`MissionRewardSkillXpGrant`** wrapper) so deny envelopes map to router reason codes. +- **Idempotent replay** — **`IRewardDeliveryStore.TryGet`** before apply; return success with existing **`RewardDeliveryEvent`** without re-granting (backlog “idempotent replay via store”). +- **Empty bundle** — null or both-empty grant lists → success with empty snapshots and **`TryRecord`** (NEO-126 allows empty arrays; CI guarantees all four prototype quests have bundles). +- **Store race** — if grants succeed but **`TryRecord`** returns `false`, retain grants and treat as idempotent success (mirror [NEO-107 encounter event note](../../../server/README.md#encounter-complete-event-record-neo-107)). + +## Goal, scope, and out-of-scope + +**Goal:** Server-internal apply for quest **`completionRewardBundle`** rows (items + skill XP) with structured outcomes and durable idempotency via **`IRewardDeliveryStore`**. + +**In scope (from Linear + [E7M2-04](E7M2-prototype-backlog.md#e7m2-04--rewardrouteroperations-apply-questrewardbundle)):** + +- **`RewardRouterOperations.TryDeliverQuestCompletion`** in `server/NeonSprawl.Server/Game/Rewards/`. +- Item grants via **`PlayerInventoryOperations`** (simulate-all pre-flight, sequential apply, compensating rollback). +- Skill XP via **`SkillProgressionGrantOperations.TryApplyGrant`** with fixed **`mission_reward`** source kind. +- **`RewardDeliveryReasonCodes`** + **`RewardDeliveryResult`** (success, deny reason, grant snapshots, **`RewardDeliveryEvent?`**). +- Unit + integration tests (AAA): happy path (skill-only + item+skill bundles), inventory deny, idempotent store replay, apply-phase item rollback, skill deny passthrough. +- `server/README.md` router section. + +**Out of scope (from Linear):** + +- Quest state transition hooks ([NEO-128](https://linear.app/neon-sprawl/issue/NEO-128) E7M2-05). +- HTTP DTOs / **`completionRewardSummary`** ([NEO-129](https://linear.app/neon-sprawl/issue/NEO-129) E7M2-06). +- E9.M1 telemetry hook comments ([NEO-130](https://linear.app/neon-sprawl/issue/NEO-130) E7M2-07). +- Godot client, Bruno (optional on NEO-128). + +## Acceptance criteria checklist + +- [ ] Bundle rows apply atomically where possible; compensating rollback on partial failure before store record. +- [ ] Skill XP uses **`mission_reward`** source kind only. + +## Technical approach + +### 1. Result + reason types (`Game/Rewards/`) + +**`RewardDeliveryReasonCodes`** — stable strings for router denies: + +| Code | When | +|------|------| +| **`already_delivered`** | **`IRewardDeliveryStore.TryGet`** hit before apply (idempotent replay — success path, not a deny). | +| **`inventory_store_missing`** | Inventory snapshot unavailable. | +| **`inventory_full`**, **`invalid_item`**, **`invalid_quantity`** | Passthrough from **`PlayerInventoryReasonCodes`** via **`PlayerInventoryOperations`**. | +| **`unknown_skill`**, **`invalid_amount`**, **`source_kind_not_allowed`** | Passthrough from **`SkillProgressionSnapshotApi`** reason constants via **`TryApplyGrant`**. | +| **`progression_store_missing`** | **`SkillProgressionGrantApplyKind.ProgressionStoreMissing`**. | + +**`RewardDeliveryResult`** — readonly record: + +| Field | Role | +|-------|------| +| **`Success`** | Grants committed (includes idempotent replay). | +| **`ReasonCode`** | Set when **`Success`** is false. | +| **`GrantedItems`** | Applied item snapshot (`RewardItemGrantApplied`). | +| **`GrantedSkillXp`** | Applied skill XP snapshot (`RewardSkillXpGrantApplied`). | +| **`DeliveryEvent`** | Populated on success (fresh or replay). | +| **`DeliveredAt`** | From event on success. | + +### 2. `RewardRouterOperations.TryDeliverQuestCompletion` + +Signature (static ops, mirror encounter completion): + +```csharp +public static RewardDeliveryResult TryDeliverQuestCompletion( + string playerId, + string questId, + QuestRewardBundleRow? bundle, + IItemDefinitionRegistry itemRegistry, + IPlayerInventoryStore inventoryStore, + ISkillDefinitionRegistry skillRegistry, + IPlayerSkillProgressionStore skillProgressionStore, + ISkillLevelCurve levelCurve, + PerkUnlockEngine perkUnlockEngine, + IRewardDeliveryStore deliveryStore, + TimeProvider timeProvider) +``` + +**Flow:** + +1. **Normalize ids** via **`RewardDeliveryIds`**; fail closed on empty player/quest id (deny with appropriate code or empty reason — match store fail-closed). +2. **Idempotent replay** — if **`deliveryStore.TryGet`**, return **`Success: true`** with existing event and grant snapshots (no inventory/skill mutation). +3. **Resolve grants** — treat null **`bundle`** as empty item + skill lists. +4. **Item pre-flight** — clone inventory snapshot; **`TrySimulateAddStack`** for each **`bundle.ItemGrants`** row in catalog order. +5. **Item apply** — foreach row **`TryAddStack`**; accumulate **`RewardItemGrantApplied`**; on deny/store-missing, return deny **without** skill apply. +6. **Skill XP apply** — foreach **`bundle.SkillXpGrants`** row, **`TryApplyGrant(playerId, skillId, amount, MissionRewardSourceKind, …)`**: + - On **`Denied`**, **`CompensatingRemoveItems`** for applied item rows and return mapped **`ReasonCode`** from **`outcome.Response!.ReasonCode`**. + - On **`ProgressionStoreMissing`**, compensating item rollback + deny. + - On **`Granted`**, append **`RewardSkillXpGrantApplied`**. +7. **Record** — build **`RewardDeliveryEvent`** with **`DeliveredAt = timeProvider.GetUtcNow()`**, **`IdempotencyKey = RewardDeliveryIds.MakeIdempotencyKey(...)`**, grant snapshots. +8. **`deliveryStore.TryRecord`** — if `true`, return fresh success; if `false` (race), return success with **`TryGet`** event (grants already applied — idempotent success). + +**Compensating item rollback** — private helper mirroring **`EncounterCompletionOperations.CompensatingRemoveGrants`**: reverse-order **`TryRemoveStack`** for applied item rows only (no skill XP undo). + +### 3. Skill XP source kind enforcement + +Always pass **`MissionRewardSkillXpConstants.MissionRewardSourceKind`** at apply time — content rows do not carry **`sourceKind`** (NEO-124/NEO-43). Startup catalog gates already require skills to allow **`mission_reward`**; router still maps **`source_kind_not_allowed`** if a deny occurs. + +### 4. Tests (`RewardRouterOperationsTests`) + +Use **`InMemoryWebApplicationFactory`** + resolved DI (same pattern as **`EncounterCompletionOperationsTests`**): + +| Test | Coverage | +|------|----------| +| **`TryDeliverQuestCompletion_ShouldApplyGatherIntroSkillXp_WhenBundleIsSkillOnly`** | **`prototype_quest_gather_intro`**-shaped bundle → salvage +25; store recorded; replay via second call returns same event without double XP. | +| **`TryDeliverQuestCompletion_ShouldApplyItemAndSkillXp_WhenOperatorChainBundle`** | **`survey_drone_kit` ×1** + salvage +50; inventory + progression assertions. | +| **`TryDeliverQuestCompletion_ShouldDenyInventoryFull_WithoutStoreRecord`** | Fill bag; deny **`inventory_full`**; **`TryGet`** false. | +| **`TryDeliverQuestCompletion_ShouldRollbackItems_WhenSkillApplyDenied`** | Item grant succeeds then skill deny (inject invalid amount or use test-only bundle row) → item removed; no store record. | +| **`TryDeliverQuestCompletion_ShouldReturnExistingEvent_WhenAlreadyDelivered`** | Pre-seed **`IRewardDeliveryStore`**; call router → success, no inventory/XP change. | +| **`TryDeliverQuestCompletion_ShouldUseMissionRewardSourceKind`** | Assert grant succeeds for catalog skill; optional negative with skill lacking allowlist (unit-level bundle row). | + +Optional host smoke: resolve **`RewardRouterOperations`** is static — assert factory provides all injected stores/registries used in one happy-path call. + +### 5. Docs + +- **`server/README.md`** — new **Reward router (NEO-127)** section: operation entry point, reason codes, apply order, idempotency + store interaction, pointer to NEO-128 wiring. +- **`documentation_and_implementation_alignment.md`** — E7.M2 row: router apply landed (NEO-127). +- **`E7_M2_RewardAndUnlockRouter.md`** — implementation anchor for **`RewardRouterOperations`**. + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/Rewards/RewardDeliveryReasonCodes.cs` | Stable router deny reason strings. | +| `server/NeonSprawl.Server/Game/Rewards/RewardDeliveryResult.cs` | Structured outcome + grant snapshots + optional event. | +| `server/NeonSprawl.Server/Game/Rewards/RewardRouterOperations.cs` | `TryDeliverQuestCompletion` apply + rollback + store record. | +| `server/NeonSprawl.Server.Tests/Game/Rewards/RewardRouterOperationsTests.cs` | AAA happy path, deny, rollback, idempotent replay tests. | +| `docs/plans/NEO-127-implementation-plan.md` | This plan. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/README.md` | Document `RewardRouterOperations`, reason codes, idempotency flow. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E7.M2 alignment — router apply (NEO-127). | +| `docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md` | Add NEO-127 implementation anchor after delivery store bullet. | + +## Tests + +| Target | Coverage | +|--------|----------| +| `server/NeonSprawl.Server.Tests/Game/Rewards/RewardRouterOperationsTests.cs` | Skill-only + item+skill happy paths; `inventory_full` fail-closed; item compensating rollback on skill deny; store idempotent replay; `mission_reward` apply path. | +| **Optional** — extend existing host/DI test | Factory resolves all router dependencies (smoke only). | +| **Manual verification** | `dotnet test server/NeonSprawl.Server.Tests --filter RewardRouterOperationsTests`; dev server boot unchanged until NEO-128 wires quest complete. | + +## Open questions / risks + +| Question or risk | Agent recommendation | Status | +|------------------|----------------------|--------| +| Multi-row skill XP partial apply | Prototype bundles have ≤1 skill row; implement sequential apply with item rollback on any skill deny; defer XP compensating rollback unless a future story requires multi-row bundles. | **adopted** | +| `MissionRewardSkillXpGrant` vs `TryApplyGrant` | Use **`TryApplyGrant`** directly for deny envelopes; keep **`MissionRewardSkillXpGrant`** as HTTP/convenience wrapper only. | **adopted** | +| Null bundle on future quests | Treat as empty grants + record delivery for idempotency; NEO-128 may skip router call when bundle absent — both safe. | **adopted** | +| Quest-state rollback on deny | NEO-128 scope per E7M2 fail-closed decision; NEO-127 returns structured deny only. | **deferred** | + +None beyond the above.