From c237bead533d33ba11a5c32df3d5838f9e6a9432 Mon Sep 17 00:00:00 2001
From: VinPropane <don@donmail.org>
Date: Sat, 27 Jun 2026 19:43:44 -0400
Subject: [PATCH] NEO-147: guard DenyCreateFailure when create snapshot is null

---
 docs/plans/NEO-147-implementation-plan.md     |  2 +-
 .../ContractGeneratorOperationsTests.cs       | 58 +++++++++++++++++++
 .../Contracts/ContractGeneratorOperations.cs  |  4 +-
 3 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/docs/plans/NEO-147-implementation-plan.md b/docs/plans/NEO-147-implementation-plan.md
index 397fccb..ae8f14d 100644
--- a/docs/plans/NEO-147-implementation-plan.md
+++ b/docs/plans/NEO-147-implementation-plan.md
@@ -70,7 +70,7 @@ Issue a validated **`active`** contract instance from **`ContractSeed`** inputs
 
 - **Types:** `ContractGeneratorReasonCodes`, `ContractIssueOperationResult`, `ContractInstanceIds.MakeDeterministicInstanceId`.
 - **Orchestrator:** `ContractGeneratorOperations.TryIssue` — band + faction template selection, deterministic id, store persist.
-- **Tests:** `ContractGeneratorOperationsTests` (13 unit), `ContractGeneratorOperationsIntegrationTests` (DI), `ContractGeneratorPersistenceIntegrationTests` (`RequirePostgresFact`); **890** tests green.
+- **Tests:** `ContractGeneratorOperationsTests` (14 unit), `ContractGeneratorOperationsIntegrationTests` (DI), `ContractGeneratorPersistenceIntegrationTests` (`RequirePostgresFact`); **891** tests green.
 - **Docs:** `server/README.md` contract generator section; `E7_M4_ContractMissionGenerator.md` NEO-147 note.
 
 ## Technical approach
diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractGeneratorOperationsTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractGeneratorOperationsTests.cs
index e296fe0..32e53d3 100644
--- a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractGeneratorOperationsTests.cs
+++ b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractGeneratorOperationsTests.cs
@@ -195,6 +195,30 @@ public sealed class ContractGeneratorOperationsTests
         Assert.False(deps.InstanceStore.TryGetActiveForPlayer(PlayerId, out _));
     }
 
+    [Fact]
+    public void TryIssue_ShouldDenyInvalidIds_WhenCreateActiveReturnsFalseWithoutSnapshot()
+    {
+        // Arrange
+        var deps = CreateDependencies();
+        var store = new CreateActiveNullSnapshotStore(deps.InstanceStore);
+        var templateRegistry = deps.TemplateRegistry;
+        var standingStore = deps.StandingStore;
+        // Act
+        var result = ContractGeneratorOperations.TryIssue(
+            PlayerId,
+            TemplateId,
+            SeedBucket,
+            zoneDifficultyBand: null,
+            templateRegistry,
+            store,
+            standingStore,
+            deps.TimeProvider);
+        // Assert
+        Assert.False(result.Success);
+        Assert.Equal(ContractGeneratorReasonCodes.InvalidIds, result.ReasonCode);
+        Assert.Null(result.Snapshot);
+    }
+
     [Fact]
     public void TryIssue_ShouldDenyPlayerNotWritable_WhenPlayerUnknown()
     {
@@ -290,4 +314,38 @@ public sealed class ContractGeneratorOperationsTests
         IContractInstanceStore InstanceStore,
         IFactionStandingStore StandingStore,
         TimeProvider TimeProvider);
+
+    /// <summary>Simulates store create failure without populating the out snapshot (Bugbot regression).</summary>
+    private sealed class CreateActiveNullSnapshotStore(IContractInstanceStore inner) : IContractInstanceStore
+    {
+        public bool CanWritePlayer(string playerId) => inner.CanWritePlayer(playerId);
+
+        public bool TryGet(string playerId, string contractInstanceId, out ContractInstanceState snapshot) =>
+            inner.TryGet(playerId, contractInstanceId, out snapshot);
+
+        public bool TryGetActiveForPlayer(string playerId, out ContractInstanceState snapshot) =>
+            inner.TryGetActiveForPlayer(playerId, out snapshot);
+
+        public bool TryCreateActive(
+            string playerId,
+            string contractInstanceId,
+            string templateId,
+            string seedBucket,
+            DateTimeOffset issuedAt,
+            out ContractInstanceState snapshot)
+        {
+            snapshot = null!;
+            return false;
+        }
+
+        public bool TryMarkComplete(
+            string playerId,
+            string contractInstanceId,
+            DateTimeOffset completedAt,
+            out ContractInstanceState snapshot) =>
+            inner.TryMarkComplete(playerId, contractInstanceId, completedAt, out snapshot);
+
+        public bool TryClearInstance(string playerId, string contractInstanceId) =>
+            inner.TryClearInstance(playerId, contractInstanceId);
+    }
 }
diff --git a/server/NeonSprawl.Server/Game/Contracts/ContractGeneratorOperations.cs b/server/NeonSprawl.Server/Game/Contracts/ContractGeneratorOperations.cs
index 8745c07..ba158b9 100644
--- a/server/NeonSprawl.Server/Game/Contracts/ContractGeneratorOperations.cs
+++ b/server/NeonSprawl.Server/Game/Contracts/ContractGeneratorOperations.cs
@@ -142,14 +142,14 @@ public static class ContractGeneratorOperations
     private static ContractIssueOperationResult DenyCreateFailure(
         IContractInstanceStore instanceStore,
         string normalizedPlayerId,
-        ContractInstanceState attemptSnapshot)
+        ContractInstanceState? attemptSnapshot)
     {
         if (instanceStore.TryGetActiveForPlayer(normalizedPlayerId, out var active))
         {
             return Deny(ContractGeneratorReasonCodes.ActiveContractExists, active);
         }
 
-        if (attemptSnapshot.Status == ContractInstanceStatus.Active)
+        if (attemptSnapshot is { Status: ContractInstanceStatus.Active })
         {
             return Deny(ContractGeneratorReasonCodes.ActiveContractExists, attemptSnapshot);
         }