NEO-127: rollback duplicate grants when TryRecord loses race

Mirror EncounterCompletionOperations compensating rollback: concurrent
losers undo item and skill XP applies before returning the winner event.

docs/plans/NEO-127-implementation-plan.md

@@ -23,7 +23,7 @@
 - **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)).
+- **Store race** — if grants succeed but **`TryRecord`** returns `false`, **rollback this call's item + skill XP grants** and return the winner's store event (mirror **`EncounterCompletionOperations`** when **`TryMarkCompleted`** loses); do not retain duplicate grants on the loser.
 
 ## Goal, scope, and out-of-scope
 
@@ -52,9 +52,9 @@
 
 ## Implementation reconciliation (shipped)
 
-- **Operations:** `RewardRouterOperations.TryDeliverQuestCompletion` — item pre-flight + apply, skill XP via `TryApplyGrant` (`mission_reward`), compensating item rollback, `IRewardDeliveryStore.TryRecord`.
+- **Operations:** `RewardRouterOperations.TryDeliverQuestCompletion` — item pre-flight + apply, skill XP via `TryApplyGrant` (`mission_reward`), compensating item + skill XP rollback on apply/skill deny and on **`TryRecord`** race loss, `IRewardDeliveryStore.TryRecord`.
 - **Types:** `RewardDeliveryResult`, `RewardDeliveryReasonCodes`.
-- **Tests:** 10 AAA tests in `RewardRouterOperationsTests` (skill-only + item+skill happy paths, inventory deny, skill deny rollback, store replay, mission_reward apply, unknown skill deny, null bundle empty record, invalid id deny).
+- **Tests:** 12 AAA tests in `RewardRouterOperationsTests` (skill-only + item+skill happy paths, inventory deny, skill deny rollback, store replay, mission_reward apply, unknown skill deny, null bundle empty record, invalid id deny, TryRecord race rollback, concurrent race no double-grant).
 - **Docs:** `server/README.md` reward router section; E7.M2 module anchor + alignment register updated.
 
 ## Technical approach
@@ -113,7 +113,7 @@ public static RewardDeliveryResult TryDeliverQuestCompletion(
    - 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).
+8. **`deliveryStore.TryRecord`** — if `true`, return fresh success; if `false` (race), **compensating rollback** of this call's item + skill XP grants, then return **`TryGet`** winner event with **`already_delivered`** (or deny if store miss).
 
 **Compensating item rollback** — private helper mirroring **`EncounterCompletionOperations.CompensatingRemoveGrants`**: reverse-order **`TryRemoveStack`** for applied item rows only (no skill XP undo).
 

server/NeonSprawl.Server.Tests/Game/Rewards/RewardRouterOperationsTests.cs

@@ -284,6 +284,88 @@ public sealed class RewardRouterOperationsTests
         Assert.False(deps.DeliveryStore.TryGet(PlayerId, "   ", out _));
     }
 
+    [Fact]
+    public async Task TryDeliverQuestCompletion_ShouldRollbackGrants_WhenTryRecordLosesRace()
+    {
+        // Arrange
+        await using var factory = new InMemoryWebApplicationFactory();
+        var deps = ResolveDependencies(factory);
+        var winnerEvent = new RewardDeliveryEvent(
+            PlayerId,
+            GatherQuestId,
+            DeliveredAt,
+            [],
+            [new RewardSkillXpGrantApplied("salvage", 25)],
+            RewardDeliveryIds.MakeIdempotencyKey(PlayerId, GatherQuestId));
+        var store = new DeliveryStoreSimulatingRecordRaceLoss(winnerEvent);
+        var bundle = new QuestRewardBundleRow([], [new QuestSkillXpGrantRow("salvage", 25)]);
+        var xpBefore = deps.SkillProgressionStore.GetXpTotals(PlayerId);
+        var salvageBefore = xpBefore.TryGetValue("salvage", out var beforeSalvage) ? beforeSalvage : 0;
+
+        // Act
+        var result = RewardRouterOperations.TryDeliverQuestCompletion(
+            PlayerId,
+            GatherQuestId,
+            bundle,
+            deps.ItemRegistry,
+            deps.InventoryStore,
+            deps.SkillRegistry,
+            deps.SkillProgressionStore,
+            deps.LevelCurve,
+            deps.PerkUnlockEngine,
+            store,
+            TimeProvider.System);
+
+        // Assert
+        Assert.True(result.Success);
+        Assert.Equal(RewardDeliveryReasonCodes.AlreadyDelivered, result.ReasonCode);
+        Assert.Equal(winnerEvent.IdempotencyKey, result.DeliveryEvent!.Value.IdempotencyKey);
+        var xpAfter = deps.SkillProgressionStore.GetXpTotals(PlayerId);
+        var salvageAfter = xpAfter.TryGetValue("salvage", out var afterSalvage) ? afterSalvage : 0;
+        Assert.Equal(salvageBefore, salvageAfter);
+    }
+
+    [Fact]
+    public async Task TryDeliverQuestCompletion_ShouldNotDoubleGrant_WhenConcurrentRecordRaces()
+    {
+        // Arrange
+        await using var factory = new InMemoryWebApplicationFactory();
+        var deps = ResolveDependencies(factory);
+        var bundle = new QuestRewardBundleRow(
+            [new RewardGrantRow("survey_drone_kit", 1)],
+            [new QuestSkillXpGrantRow("salvage", 25)]);
+        deps.InventoryStore.TryGetSnapshot(PlayerId, out var inventoryBefore);
+        var xpBefore = deps.SkillProgressionStore.GetXpTotals(PlayerId);
+        var salvageBefore = xpBefore.TryGetValue("salvage", out var beforeSalvage) ? beforeSalvage : 0;
+        var kitBefore = CountBagItem(inventoryBefore!, "survey_drone_kit");
+        var barrier = new Barrier(2);
+        RewardDeliveryResult result1 = default;
+        RewardDeliveryResult result2 = default;
+
+        // Act
+        await Task.WhenAll(
+            Task.Run(() =>
+            {
+                barrier.SignalAndWait();
+                result1 = Deliver(deps, OperatorChainQuestId, bundle, TimeProvider.System);
+            }),
+            Task.Run(() =>
+            {
+                barrier.SignalAndWait();
+                result2 = Deliver(deps, OperatorChainQuestId, bundle, TimeProvider.System);
+            }));
+
+        // Assert
+        Assert.True(result1.Success);
+        Assert.True(result2.Success);
+        deps.InventoryStore.TryGetSnapshot(PlayerId, out var inventoryAfter);
+        var xpAfter = deps.SkillProgressionStore.GetXpTotals(PlayerId);
+        Assert.True(xpAfter.TryGetValue("salvage", out var salvageAfter));
+        Assert.Equal(salvageBefore + 25, salvageAfter);
+        Assert.Equal(kitBefore + 1, CountBagItem(inventoryAfter!, "survey_drone_kit"));
+        Assert.True(deps.DeliveryStore.TryGet(PlayerId, OperatorChainQuestId, out _));
+    }
+
     private static RewardDeliveryResult Deliver(
         RewardRouterTestDependencies deps,
         string questId,
@@ -365,4 +447,28 @@ public sealed class RewardRouterOperationsTests
         ISkillLevelCurve LevelCurve,
         PerkUnlockEngine PerkUnlockEngine,
         IRewardDeliveryStore DeliveryStore);
+
+    /// <summary>TryGet misses until after a failed <see cref="IRewardDeliveryStore.TryRecord"/> — simulates concurrent race loser.</summary>
+    private sealed class DeliveryStoreSimulatingRecordRaceLoss(RewardDeliveryEvent winnerEvent) : IRewardDeliveryStore
+    {
+        private bool recordAttempted;
+
+        public bool TryRecord(RewardDeliveryEvent deliveryEvent)
+        {
+            recordAttempted = true;
+            return false;
+        }
+
+        public bool TryGet(string playerId, string questId, out RewardDeliveryEvent deliveryEvent)
+        {
+            if (!recordAttempted)
+            {
+                deliveryEvent = default;
+                return false;
+            }
+
+            deliveryEvent = winnerEvent;
+            return true;
+        }
+    }
 }

server/NeonSprawl.Server/Game/Rewards/RewardRouterOperations.cs

@@ -141,12 +141,17 @@ public static class RewardRouterOperations
             return SuccessFromEvent(deliveryEvent, reasonCode: null);
         }
 
+        // Lost TryRecord race after apply — rollback this call's grants (mirror
+        // EncounterCompletionOperations when TryMarkCompleted returns false).
+        CompensatingRemoveItems(playerId, appliedItems, itemRegistry, inventoryStore);
+        CompensatingRemoveSkillXp(playerId, appliedSkillXp, skillProgressionStore);
+
         if (deliveryStore.TryGet(normalizedPlayerId, normalizedQuestId, out var racedEvent))
         {
             return SuccessFromEvent(racedEvent, RewardDeliveryReasonCodes.AlreadyDelivered);
         }
 
-        return SuccessFromEvent(deliveryEvent, RewardDeliveryReasonCodes.AlreadyDelivered);
+        return Deny(RewardDeliveryReasonCodes.AlreadyDelivered);
     }
 
     private static string MapInventoryReason(string? reasonCode) =>
@@ -177,6 +182,23 @@ public static class RewardRouterOperations
         }
     }
 
+    private static void CompensatingRemoveSkillXp(
+        string playerId,
+        IReadOnlyList<RewardSkillXpGrantApplied> grants,
+        IPlayerSkillProgressionStore skillProgressionStore)
+    {
+        for (var i = grants.Count - 1; i >= 0; i--)
+        {
+            var grant = grants[i];
+            _ = skillProgressionStore.TryApplyXpDelta(
+                playerId,
+                grant.SkillId,
+                -grant.Amount,
+                out _,
+                out _);
+        }
+    }
+
     private static RewardDeliveryResult Deny(string reasonCode) =>
         new(
             Success: false,

server/README.md

@@ -297,7 +297,7 @@ Encounter loot remains on **`IEncounterCompleteEventStore`** (E5.M3); quest comp
 | **`unknown_skill`**, **`invalid_amount`**, **`source_kind_not_allowed`** | Skill XP grant denied. |
 | **`progression_store_missing`** | Skill progression store could not apply XP delta. |
 
-Apply order: **items first**, then **skill XP**. Inventory deny or skill deny fails closed without store record (compensating item rollback on partial apply or skill failure after items). Idempotent replay checks **`TryGet`** before apply. If grants succeed but **`TryRecord`** returns `false` (race), grants are retained and the existing store event is returned. Quest-state wiring lands in NEO-128. Plan: [NEO-127 implementation plan](../../docs/plans/NEO-127-implementation-plan.md).
+Apply order: **items first**, then **skill XP**. Inventory deny or skill deny fails closed without store record (compensating item rollback on partial apply or skill failure after items). Idempotent replay checks **`TryGet`** before apply. If grants succeed but **`TryRecord`** returns `false` (concurrent race), this call **rolls back** its item + skill XP grants and returns the existing store event (mirror **`EncounterCompletionOperations`** compensating rollback when **`TryMarkCompleted`** loses). Quest-state wiring lands in NEO-128. Plan: [NEO-127 implementation plan](../../docs/plans/NEO-127-implementation-plan.md).
 
 ## Encounter definitions (NEO-103)