neon-sprawl/docs/plans/NEO-148-implementation-plan.md

171 lines
13 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters!

This file contains ambiguous Unicode characters that may be confused with others in your current locale. If your use case is intentional and legitimate, you can safely ignore this warning. Use the Escape button to highlight these characters.

# NEO-148 — E7M4-05: Extend RewardRouterOperations for contract completion bundles
**Linear:** [NEO-148](https://linear.app/neon-sprawl/issue/NEO-148)
**Branch:** `NEO-148-e7m4-05-reward-router-contract-completion`
**Backlog:** [E7M4-pre-production-backlog.md](E7M4-pre-production-backlog.md) — **E7M4-05**
**Module:** [E7_M4_ContractMissionGenerator.md](../decomposition/modules/E7_M4_ContractMissionGenerator.md)
**Pattern:** [NEO-127-implementation-plan.md](NEO-127-implementation-plan.md) — grant order + compensating rollback + `TryRecord` race mirror; [NEO-138-implementation-plan.md](NEO-138-implementation-plan.md) — reputation grants + store snapshot extension
**Precursors:** [NEO-145](https://linear.app/neon-sprawl/issue/NEO-145) **`Done`** — contract template catalog load; [NEO-146](https://linear.app/neon-sprawl/issue/NEO-146) **`Done`** — `ContractOutcomeIds.MakeIdempotencyKey`, grant snapshot types
**Blocks:** [NEO-149](https://linear.app/neon-sprawl/issue/NEO-149) (E7M4-06 `ContractCompletionOperations` + encounter wiring)
**Client counterpart:** [NEO-153](https://linear.app/neon-sprawl/issue/NEO-153) (blocked by E7M4-08; no Godot work in this story)
## Goal
Contract completion payouts reuse E7.M2 delivery idempotency with contract-specific keys (`{playerId}:contract_complete:{contractInstanceId}`).
## Kickoff clarifications
**No blocking decisions needed.** Linear AC, [E7M4-05 backlog scope](E7M4-pre-production-backlog.md#e7m4-05--extend-rewardrouteroperations-for-contract-completion-bundles), [E7M4 kickoff decisions table](E7M4-pre-production-backlog.md#kickoff-decisions-decomposition-defaults), and landed NEO-127/NEO-138/NEO-146 code settle:
| Topic | Decision | Evidence |
|-------|----------|----------|
| Idempotency key | **`ContractOutcomeIds.MakeIdempotencyKey`** → `{playerId}:contract_complete:{contractInstanceId}` | E7M4 kickoff table; NEO-146 shipped helper |
| Bundle DTO | Reuse **`QuestRewardBundleRow`** on **`ContractTemplateRow.CompletionRewardBundle`** | NEO-145 loader; prototype freeze table |
| Grant kinds + order | Items → skill XP → reputation; compensating rollback on partial deny and **`TryRecord`** race loss | E7M4-05 backlog “mirrors E7.M2 quest path”; NEO-127/NEO-138 |
| Delivery store | Extend **`IRewardDeliveryStore`** / **`RewardDeliveryEvent`** with **`contract_completion`** source kind | E7M4-05 backlog in-scope list |
| Outcome audit store | **Out of scope****`IContractOutcomeStore.TryAppend`** is E7M4-06 orchestrator responsibility | NEO-146 open-questions table (“orchestrator fills from router result”) |
| Reputation source | **`ReputationDeltaSourceKinds.ContractCompletion`** + **`contractInstanceId`** as **`sourceId`** | NEO-138 **`QuestCompletion`** precedent |
| Skill XP source kind | **`mission_reward`** (same as quest completion) | Prototype contract freeze; NEO-127 |
| Router shape | **`TryDeliverContractCompletion`** public entry + private shared grant-apply core extracted from quest path | Avoid ~200-line duplication while keeping **`TryDeliverQuestCompletion`** signature stable |
| Store API | Source-kind-aware **`TryGet`/`TryRecord`/`TryClear`**; existing quest methods remain thin wrappers | Backlog “extend store for contract source kind”; backward compat for NEO-126 callers |
| **`IContractOutcomeStore` replay** | Router idempotency uses **`IRewardDeliveryStore`** only; outcome store is separate audit trail | E7M4-05 vs E7M4-06 scope split |
| Postgres delivery store | **Deferred** — in-memory only (same as quest delivery today) | NEO-126/NEO-138 precedent |
| Encounter / HTTP wiring | **Out of scope** | E7M4-06, E7M4-08 |
## Scope and out-of-scope
**In scope (from Linear + backlog):**
- **`RewardRouterOperations.TryDeliverContractCompletion`** — same grant kinds as quest completion with contract idempotency key.
- Extend **`IRewardDeliveryStore`**, **`RewardDeliveryEvent`**, **`RewardDeliveryIds`**, **`InMemoryRewardDeliveryStore`** for **`contract_completion`** source kind.
- Add **`ReputationDeltaSourceKinds.ContractCompletion`**.
- Compensating rollback policy mirrors E7.M2 quest path (items, skill XP, reputation, perk revert, race loser).
- Integration tests: first delivery, idempotent replay, unknown item/faction deny.
- `server/README.md` reward router + delivery store sections.
**Out of scope (from Linear + backlog):**
- **`ContractCompletionOperations`** + encounter wiring (NEO-149 / E7M4-06).
- **`IContractOutcomeStore.TryAppend`** from router (orchestrator in E7M4-06).
- HTTP contract projections (NEO-151 / E7M4-08).
- E9.M1 telemetry ingest (comment-only hook sites optional; not required by AC).
- Godot client (NEO-153+).
**Client counterpart:** [NEO-153](https://linear.app/neon-sprawl/issue/NEO-153) — contract HUD after E7M4-08.
## Acceptance criteria checklist
- [x] First-time contract complete applies bundle exactly once per instance id.
- [x] Replay returns **`already_delivered`** without double grant.
## Implementation reconciliation (shipped)
- **Types:** `RewardDeliverySourceKinds`; `RewardDeliveryEvent.SourceKind` + `ContractInstanceId`; `RewardDeliveryReasonCodes.InvalidContractInstanceId`; `ReputationDeltaSourceKinds.ContractCompletion`.
- **Store:** `IRewardDeliveryStore` source-kind-aware `TryGet`/`TryClear`; `InMemoryRewardDeliveryStore` keys `{playerId}\0{sourceKind}\0{sourceId}`.
- **Router:** `TryDeliverContractCompletion` + private shared `TryDeliverBundle` core; quest path unchanged at public API.
- **Tests:** 5 contract router tests + 1 store isolation test; full suite **897** tests green.
- **Docs:** `server/README.md` delivery store + router sections; E7.M4 module anchor.
## Technical approach
### 1. Types (`Game/Rewards/`)
| Type | Change |
|------|--------|
| **`RewardDeliverySourceKinds`** (new) | Stable strings: **`quest_completion`**, **`contract_completion`** |
| **`RewardDeliveryEvent`** | Add **`SourceKind`**; add **`ContractInstanceId`** (empty for quest rows); quest rows keep **`QuestId`** populated |
| **`RewardDeliveryIds`** | Add **`NormalizeContractInstanceId`** (delegate to **`ContractInstanceIds`**); **`MakeContractStoreKey(playerId, contractInstanceId)`**; extend **`MakeStoreKey`** to include source kind `{p}\0{kind}\0{id}` |
| **`RewardDeliveryReasonCodes`** | Add **`invalid_contract_instance_id`** |
| **`IRewardDeliveryStore`** | Add **`TryGet(playerId, sourceKind, sourceId, …)`**, **`TryRecord`**, **`TryClear`** overloads; keep existing quest methods as wrappers calling **`quest_completion`** |
| **`InMemoryRewardDeliveryStore`** | Index by source-kind-aware store key; quest + contract rows isolated |
Add **`ReputationDeltaSourceKinds.ContractCompletion = "contract_completion"`** in `Game/Factions/`.
### 2. `RewardRouterOperations`
**New public method:**
```csharp
public static RewardDeliveryResult TryDeliverContractCompletion(
string playerId,
string contractInstanceId,
QuestRewardBundleRow? bundle,
/* same dependency injection as TryDeliverQuestCompletion */)
```
**Flow** (mirror quest):
1. Normalize **`playerId`** + **`contractInstanceId`**; deny on empty ids.
2. **`deliveryStore.TryGet(playerId, contract_completion, contractInstanceId)`** — idempotent replay → **`already_delivered`** success without re-grant.
3. Resolve bundle grant lists (null → empty).
4. Apply items → skill XP → reputation with **`ContractOutcomeIds.MakeIdempotencyKey`** and **`ReputationDeltaSourceKinds.ContractCompletion`** + **`contractInstanceId`** as rep **`sourceId`**.
5. Build **`RewardDeliveryEvent`** with **`SourceKind = contract_completion`**, **`ContractInstanceId`**, empty **`QuestId`**, grant snapshots, idempotency key.
6. **`TryRecord`**; on race loss, compensating rollback all grant types and return winner event.
**Refactor:** Extract private **`TryDeliverBundleCore`** (or equivalent) parameterized by delivery context (source kind, source id, idempotency key factory, rep source kind/id) so quest and contract paths share rollback/apply logic. **`TryDeliverQuestCompletion`** remains a thin wrapper — no signature change for callers.
### 3. Prototype test fixtures
Use frozen template bundle from **`prototype_contract_clear_combat_pocket`**:
- **`scrap_metal_bulk` ×5**, **`salvage` 15 XP** (no rep in prototype contract).
Contract instance id: synthetic test value (e.g. **`prototype_contract_clear_combat_pocket:test-seed-001`**) — no generator dependency required for router unit tests.
### 4. Documentation
- **`server/README.md`**: extend reward delivery store table with **`SourceKind`** + contract key; add **`TryDeliverContractCompletion`** section under reward router with reason codes (reuse quest passthrough set + **`invalid_contract_instance_id`**).
- Update **`E7_M4_ContractMissionGenerator.md`** Slice 4 anchor when shipped (router landed).
## Files to add
| Path | Purpose |
|------|---------|
| `server/NeonSprawl.Server/Game/Rewards/RewardDeliverySourceKinds.cs` | Stable **`quest_completion`** / **`contract_completion`** source kind constants |
*(No other new source files expected — router + store extensions land in existing files.)*
## Files to modify
| Path | Rationale |
|------|-----------|
| `server/NeonSprawl.Server/Game/Rewards/RewardRouterOperations.cs` | Add **`TryDeliverContractCompletion`**; extract shared grant-apply core |
| `server/NeonSprawl.Server/Game/Rewards/IRewardDeliveryStore.cs` | Source-kind-aware **`TryGet`/`TryRecord`/`TryClear`** overloads |
| `server/NeonSprawl.Server/Game/Rewards/RewardDeliveryEvent.cs` | Add **`SourceKind`**, **`ContractInstanceId`** |
| `server/NeonSprawl.Server/Game/Rewards/RewardDeliveryIds.cs` | Contract instance normalization + source-kind store keys |
| `server/NeonSprawl.Server/Game/Rewards/RewardDeliveryReasonCodes.cs` | **`invalid_contract_instance_id`** |
| `server/NeonSprawl.Server/Game/Rewards/InMemoryRewardDeliveryStore.cs` | Source-kind-aware indexing |
| `server/NeonSprawl.Server/Game/Factions/ReputationDeltaSourceKinds.cs` | **`ContractCompletion`** constant |
| `server/NeonSprawl.Server.Tests/Game/Rewards/RewardRouterOperationsTests.cs` | Contract delivery happy path, replay, deny tests |
| `server/NeonSprawl.Server.Tests/Game/Rewards/InMemoryRewardDeliveryStoreTests.cs` | Contract source kind idempotency at store layer |
| `server/README.md` | Delivery store + router contract completion docs |
| `docs/decomposition/modules/E7_M4_ContractMissionGenerator.md` | Slice 4 router anchor when shipped |
## Tests
| File | Coverage |
|------|----------|
| `server/NeonSprawl.Server.Tests/Game/Rewards/RewardRouterOperationsTests.cs` | **`TryDeliverContractCompletion_ShouldApplyPrototypeBundle_WhenFirstDelivery`** — items + skill XP applied once; store record with **`contract_completion`** key |
| Same | **`TryDeliverContractCompletion_ShouldReturnAlreadyDelivered_WhenReplayed`** — second call success + **`already_delivered`**; inventory/xp unchanged |
| Same | **`TryDeliverContractCompletion_ShouldDenyUnknownFaction_WithoutStoreRecord`** — rep grant with unknown faction; no delivery row |
| Same | **`TryDeliverContractCompletion_ShouldDenyInventoryFull_WithoutStoreRecord`** — unknown item / full bag deny (backlog “unknown item/faction deny”) |
| Same | **`TryDeliverContractCompletion_ShouldDenyInvalidContractInstanceId_WhenEmpty`** — fail-closed id validation |
| `server/NeonSprawl.Server.Tests/Game/Rewards/InMemoryRewardDeliveryStoreTests.cs` | Contract **`TryRecord`** idempotency + source-kind isolation from quest rows |
All new tests follow AAA with **`// Arrange` / `// Act` / `// Assert`** labels per [csharp-style](../.cursor/rules/csharp-style.md).
Manual Godot QA: **none** (server engine; capstone NEO-154).
## Open questions / risks
| Question / risk | Agent recommendation | Status |
|-----------------|----------------------|--------|
| Shared-core refactor touches quest path | Keep **`TryDeliverQuestCompletion`** as thin wrapper; run full existing **`RewardRouterOperationsTests`** suite unchanged | `adopted` |
| Quest vs contract store key collision | Prefix store key with **`SourceKind`** — ids already namespaced (`prototype_quest_*` vs generated instance ids) | `adopted` |
| Rep grants on future contract templates | Apply path ready; prototype template has no rep rows — test unknown-faction deny with synthetic bundle | `adopted` |
| **`IContractOutcomeStore`** vs delivery store duplication | Delivery store = grant idempotency (E7.M2 pattern); outcome store = audit append from E7M4-06 orchestrator only | `adopted` |
## Client counterpart
[NEO-153](https://linear.app/neon-sprawl/issue/NEO-153) — contract panel after E7M4-08 HTTP. This story is server-only; Bruno/HTTP contract loop verification lands in E7M4-06+.