NEO-127: add E7M2-04 reward router operations implementation plan
Kickoff plan for RewardRouterOperations apply path building on NEO-125 catalog bundles and NEO-126 delivery store idempotency.pull/166/head
parent
ce142b9271
commit
35b8904f15
|
|
@ -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.
|
||||
Loading…
Reference in New Issue