diff --git a/docs/plans/NEO-116-implementation-plan.md b/docs/plans/NEO-116-implementation-plan.md
index 105a0bf..b992196 100644
--- a/docs/plans/NEO-116-implementation-plan.md
+++ b/docs/plans/NEO-116-implementation-plan.md
@@ -60,7 +60,8 @@
- **Types:** `QuestProgressStatus`, `QuestStepState`, `QuestProgressIds`.
- **Store:** `IPlayerQuestStateStore`; `InMemoryPlayerQuestStateStore` (dev player seeded); `PostgresPlayerQuestStateStore` + `V008__player_quest_progress.sql` when Postgres configured.
- **DI:** `AddPlayerQuestStateStore` in `Program.cs`; in-memory override in `InMemoryWebApplicationFactory`.
-- **Tests:** `InMemoryPlayerQuestStateStoreTests` (10 AAA cases + host DI); `PlayerQuestProgressPersistenceIntegrationTests` (`RequirePostgresFact`).
+- **Tests:** `InMemoryPlayerQuestStateStoreTests` (AAA cases + host DI + empty-id denial); `PlayerQuestProgressPersistenceIntegrationTests` (`RequirePostgresFact`).
+- **Bruno:** `bruno/neon-sprawl-server/quest-progress/Health after quest state store load.bru` — health smoke only (pre-commit hook when `Program.cs` changes; no quest HTTP this story).
- **Docs:** `server/README.md` quest progress store section; alignment register E7.M1 row updated.
## Technical approach
diff --git a/docs/reviews/2026-06-03-NEO-116.md b/docs/reviews/2026-06-03-NEO-116.md
index 4bf5890..36e89a8 100644
--- a/docs/reviews/2026-06-03-NEO-116.md
+++ b/docs/reviews/2026-06-03-NEO-116.md
@@ -32,23 +32,23 @@ NEO-116 adds **`IPlayerQuestStateStore`** with thread-safe **`InMemoryPlayerQues
## Suggestions
-1. **Persistence integration test AAA layout** — `PlayerQuestProgressPersistenceIntegrationTests.ActivateCounterComplete_ShouldPersistAcrossNewFactory` labels store writes as `// Act` inside the `// Arrange` block. Match **`PlayerInventoryPersistenceIntegrationTests`**: reset in Arrange; first `using` scope = Act (write); second factory scope = Act (read); Assert last.
+1. ~~**Persistence integration test AAA layout** — `PlayerQuestProgressPersistenceIntegrationTests.ActivateCounterComplete_ShouldPersistAcrossNewFactory` labels store writes as `// Act` inside the `// Arrange` block. Match **`PlayerInventoryPersistenceIntegrationTests`**: reset in Arrange; first `using` scope = Act (write); second factory scope = Act (read); Assert last.~~ **Done.** Persistence test now uses separate Arrange / Act (write) / Act (read) / Assert phases.
-2. **Empty-id denial tests** — Implementation plan test table lists “unknown player / empty ids false”; only unknown player is covered. Add a `[Fact]` (or theory) asserting `TryActivate` / `TryGetProgress` return false for empty/whitespace `playerId` or `questId`.
+2. ~~**Empty-id denial tests** — Implementation plan test table lists “unknown player / empty ids false”; only unknown player is covered. Add a `[Fact]` (or theory) asserting `TryActivate` / `TryGetProgress` return false for empty/whitespace `playerId` or `questId`.~~ **Done.** `[Theory]` with four empty/whitespace player+quest id cases added to **`InMemoryPlayerQuestStateStoreTests`**.
-3. **Frozen quest id constants in tests** — Plan adopted reusing **`PrototypeE7M1QuestCatalogRules`**; tests use **`ChainQuestId`** but hardcode **`prototype_quest_gather_intro`** and objective ids. A **`GatherIntroQuestId`** constant (parallel to **`ChainQuestId`**) would avoid drift; optional helper for first-step objective ids if reused in NEO-117.
+3. ~~**Frozen quest id constants in tests** — Plan adopted reusing **`PrototypeE7M1QuestCatalogRules`**; tests use **`ChainQuestId`** but hardcode **`prototype_quest_gather_intro`** and objective ids. A **`GatherIntroQuestId`** constant (parallel to **`ChainQuestId`**) would avoid drift; optional helper for first-step objective ids if reused in NEO-117.~~ **Done.** Added **`GatherIntroQuestId`**, **`GatherIntroFirstObjectiveId`**, and **`ChainFirstObjectiveId`** to **`PrototypeE7M1QuestCatalogRules`**; tests updated.
-4. **Plan reconciliation for Bruno** — Kickoff/plan **out of scope** lists Bruno; branch adds `bruno/neon-sprawl-server/quest-progress/Health after quest state store load.bru` (health smoke only). Harmless, but add a one-line note under **Implementation reconciliation** so the plan matches shipped artifacts.
+4. ~~**Plan reconciliation for Bruno** — Kickoff/plan **out of scope** lists Bruno; branch adds `bruno/neon-sprawl-server/quest-progress/Health after quest state store load.bru` (health smoke only). Harmless, but add a one-line note under **Implementation reconciliation** so the plan matches shipped artifacts.~~ **Done.** Bruno health smoke noted in plan reconciliation and **`server/README.md`**.
## Nits
-- Nit: **`IPlayerQuestStateStore`** XML `` references a type not yet in the repo (NEO-117). Harmless for compile; consider `QuestStateOperations` until NEO-117 lands if doc warnings appear.
+- ~~Nit: **`IPlayerQuestStateStore`** XML `` references a type not yet in the repo (NEO-117). Harmless for compile; consider `QuestStateOperations` until NEO-117 lands if doc warnings appear.~~ **Done.**
-- Nit: **`PostgresPlayerQuestStateStore.TryActivate`** returns false on duplicate row without explicit **`tx.Rollback()`** (relies on dispose). **`PostgresPlayerGigProgressionStore`** rolls back explicitly on failure paths — aligning would improve consistency.
+- ~~Nit: **`PostgresPlayerQuestStateStore.TryActivate`** returns false on duplicate row without explicit **`tx.Rollback()`** (relies on dispose). **`PostgresPlayerGigProgressionStore`** rolls back explicitly on failure paths — aligning would improve consistency.~~ **Done.**
-- Nit: **`ReadSnapshot`** uses **`reader.GetString(2)`** for JSONB counters; works if Npgsql maps JSONB as string (integration test validates when Postgres CI runs). If driver behavior changes, prefer **`GetFieldValue`** or typed JSON read — same pattern worth watching in future JSONB stores.
+- ~~Nit: **`ReadSnapshot`** uses **`reader.GetString(2)`** for JSONB counters; works if Npgsql maps JSONB as string (integration test validates when Postgres CI runs). If driver behavior changes, prefer **`GetFieldValue`** or typed JSON read — same pattern worth watching in future JSONB stores.~~ **Done.** Uses **`GetFieldValue(2)`**.
-- Nit: Bruno health folder is useful startup smoke but not linked from `server/README.md` (unlike NEO-115 quest-definitions Bruno link). Optional README mention when HTTP lands in E7M1-08.
+- ~~Nit: Bruno health folder is useful startup smoke but not linked from `server/README.md` (unlike NEO-115 quest-definitions Bruno link). Optional README mention when HTTP lands in E7M1-08.~~ **Done.** README links **`bruno/neon-sprawl-server/quest-progress/`** (health smoke until E7M1-08).
## Verification
diff --git a/server/NeonSprawl.Server.Tests/Game/Quests/InMemoryPlayerQuestStateStoreTests.cs b/server/NeonSprawl.Server.Tests/Game/Quests/InMemoryPlayerQuestStateStoreTests.cs
index 12d0645..a741451 100644
--- a/server/NeonSprawl.Server.Tests/Game/Quests/InMemoryPlayerQuestStateStoreTests.cs
+++ b/server/NeonSprawl.Server.Tests/Game/Quests/InMemoryPlayerQuestStateStoreTests.cs
@@ -10,9 +10,9 @@ public sealed class InMemoryPlayerQuestStateStoreTests
{
private const string PlayerId = "dev-local-1";
private const string UnknownPlayerId = "unknown-player-xyz";
- private const string GatherQuestId = "prototype_quest_gather_intro";
+ private const string GatherQuestId = PrototypeE7M1QuestCatalogRules.GatherIntroQuestId;
private const string ChainQuestId = PrototypeE7M1QuestCatalogRules.ChainQuestId;
- private const string GatherObjectiveId = "gather_intro_obj_scrap";
+ private const string GatherObjectiveId = PrototypeE7M1QuestCatalogRules.GatherIntroFirstObjectiveId;
private static readonly DateTimeOffset CompletedAt = new(2026, 6, 3, 12, 0, 0, TimeSpan.Zero);
[Fact]
@@ -73,7 +73,7 @@ public sealed class InMemoryPlayerQuestStateStoreTests
// Arrange
var store = CreateStore();
Assert.True(store.TryActivate(PlayerId, ChainQuestId, out _));
- Assert.True(store.TryUpdateObjectiveCounter(PlayerId, ChainQuestId, "chain_obj_gather", 5, out _));
+ Assert.True(store.TryUpdateObjectiveCounter(PlayerId, ChainQuestId, PrototypeE7M1QuestCatalogRules.ChainFirstObjectiveId, 5, out _));
// Act
var advanced = store.TryAdvanceStep(PlayerId, ChainQuestId, 1, out var snapshot);
// Assert
@@ -141,6 +141,25 @@ public sealed class InMemoryPlayerQuestStateStoreTests
Assert.False(activated);
}
+ [Theory]
+ [InlineData("", GatherQuestId)]
+ [InlineData(" ", GatherQuestId)]
+ [InlineData(PlayerId, "")]
+ [InlineData(PlayerId, " ")]
+ public void TryGetProgressAndActivate_ShouldReturnFalse_WhenPlayerOrQuestIdEmpty(
+ string playerId,
+ string questId)
+ {
+ // Arrange
+ var store = CreateStore();
+ // Act
+ var getProgress = store.TryGetProgress(playerId, questId, out _);
+ var activate = store.TryActivate(playerId, questId, out _);
+ // Assert
+ Assert.False(getProgress);
+ Assert.False(activate);
+ }
+
[Fact]
public async Task Host_ShouldResolveStore_AndActivateGatherIntro()
{
diff --git a/server/NeonSprawl.Server.Tests/Game/Quests/PlayerQuestProgressPersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Quests/PlayerQuestProgressPersistenceIntegrationTests.cs
index 8b21b7d..d405783 100644
--- a/server/NeonSprawl.Server.Tests/Game/Quests/PlayerQuestProgressPersistenceIntegrationTests.cs
+++ b/server/NeonSprawl.Server.Tests/Game/Quests/PlayerQuestProgressPersistenceIntegrationTests.cs
@@ -14,8 +14,6 @@ public sealed class PlayerQuestProgressPersistenceIntegrationTests(PostgresInteg
private PostgresWebApplicationFactory Factory => harness.Factory;
private const string PlayerId = "dev-local-1";
- private const string GatherQuestId = "prototype_quest_gather_intro";
- private const string GatherObjectiveId = "gather_intro_obj_scrap";
private static readonly DateTimeOffset CompletedAt = new(2026, 6, 3, 14, 30, 0, TimeSpan.Zero);
[RequirePostgresFact]
@@ -23,28 +21,44 @@ public sealed class PlayerQuestProgressPersistenceIntegrationTests(PostgresInteg
{
// Arrange
await ResetQuestProgressTableAsync();
- using (var scope = Factory.Services.CreateScope())
+
+ // Act — write through first host
+ using (var firstScope = Factory.Services.CreateScope())
{
- var store = scope.ServiceProvider.GetRequiredService();
- // Act — write on first host
- Assert.True(store.TryActivate(PlayerId, GatherQuestId, out _));
- Assert.True(store.TryUpdateObjectiveCounter(PlayerId, GatherQuestId, GatherObjectiveId, 3, out _));
- Assert.True(store.TryMarkComplete(PlayerId, GatherQuestId, CompletedAt, out _));
+ var store = firstScope.ServiceProvider.GetRequiredService();
+ Assert.True(store.TryActivate(PlayerId, PrototypeE7M1QuestCatalogRules.GatherIntroQuestId, out _));
+ Assert.True(store.TryUpdateObjectiveCounter(
+ PlayerId,
+ PrototypeE7M1QuestCatalogRules.GatherIntroQuestId,
+ PrototypeE7M1QuestCatalogRules.GatherIntroFirstObjectiveId,
+ 3,
+ out _));
+ Assert.True(store.TryMarkComplete(
+ PlayerId,
+ PrototypeE7M1QuestCatalogRules.GatherIntroQuestId,
+ CompletedAt,
+ out _));
}
+ // Act — read back through a fresh host
QuestStepState readBack;
await using (var secondFactory = new PostgresWebApplicationFactory())
{
- using var scope = secondFactory.Services.CreateScope();
- var store = scope.ServiceProvider.GetRequiredService();
- readBack = store.TryGetProgress(PlayerId, GatherQuestId, out var snapshot) ? snapshot : null!;
+ using var secondScope = secondFactory.Services.CreateScope();
+ var store = secondScope.ServiceProvider.GetRequiredService();
+ readBack = store.TryGetProgress(
+ PlayerId,
+ PrototypeE7M1QuestCatalogRules.GatherIntroQuestId,
+ out var snapshot)
+ ? snapshot
+ : null!;
}
// Assert
Assert.NotNull(readBack);
Assert.Equal(QuestProgressStatus.Completed, readBack.Status);
Assert.Equal(0, readBack.CurrentStepIndex);
- Assert.Equal(3, readBack.ObjectiveCounters[GatherObjectiveId]);
+ Assert.Equal(3, readBack.ObjectiveCounters[PrototypeE7M1QuestCatalogRules.GatherIntroFirstObjectiveId]);
Assert.Equal(CompletedAt, readBack.CompletedAt);
}
diff --git a/server/NeonSprawl.Server/Game/Quests/IPlayerQuestStateStore.cs b/server/NeonSprawl.Server/Game/Quests/IPlayerQuestStateStore.cs
index b1276f6..53342d4 100644
--- a/server/NeonSprawl.Server/Game/Quests/IPlayerQuestStateStore.cs
+++ b/server/NeonSprawl.Server/Game/Quests/IPlayerQuestStateStore.cs
@@ -2,7 +2,7 @@ namespace NeonSprawl.Server.Game.Quests;
///
/// Persisted per-player quest progress keyed by (playerId, questId) (NEO-116).
-/// Missing row ⇒ not_started. (NEO-117) and HTTP (E7M1-08) consume this interface.
+/// Missing row ⇒ not_started. QuestStateOperations (NEO-117) and HTTP (E7M1-08) consume this interface.
///
public interface IPlayerQuestStateStore
{
diff --git a/server/NeonSprawl.Server/Game/Quests/PostgresPlayerQuestStateStore.cs b/server/NeonSprawl.Server/Game/Quests/PostgresPlayerQuestStateStore.cs
index 22e0ef7..135c828 100644
--- a/server/NeonSprawl.Server/Game/Quests/PostgresPlayerQuestStateStore.cs
+++ b/server/NeonSprawl.Server/Game/Quests/PostgresPlayerQuestStateStore.cs
@@ -85,6 +85,7 @@ public sealed class PostgresPlayerQuestStateStore(Npgsql.NpgsqlDataSource dataSo
if (reader.Read())
{
snapshot = ReadSnapshot(reader, player, quest);
+ tx.Rollback();
return false;
}
}
@@ -304,7 +305,7 @@ public sealed class PostgresPlayerQuestStateStore(Npgsql.NpgsqlDataSource dataSo
{
var status = ParseStatus(reader.GetString(0));
var stepIndex = reader.GetInt32(1);
- var countersJson = reader.GetString(2);
+ var countersJson = reader.GetFieldValue(2);
var completedAt = reader.IsDBNull(3) ? (DateTimeOffset?)null : reader.GetFieldValue(3);
var counters = DeserializeCounters(countersJson);
return new QuestStepState(player, quest, status, stepIndex, counters, completedAt);
diff --git a/server/NeonSprawl.Server/Game/Quests/PrototypeE7M1QuestCatalogRules.cs b/server/NeonSprawl.Server/Game/Quests/PrototypeE7M1QuestCatalogRules.cs
index d76ac3c..1de299f 100644
--- a/server/NeonSprawl.Server/Game/Quests/PrototypeE7M1QuestCatalogRules.cs
+++ b/server/NeonSprawl.Server/Game/Quests/PrototypeE7M1QuestCatalogRules.cs
@@ -23,6 +23,15 @@ public static class PrototypeE7M1QuestCatalogRules
/// Keep in sync with scripts/validate_content.py PROTOTYPE_E7M1_CHAIN_QUEST_ID.
public const string ChainQuestId = "prototype_quest_operator_chain";
+ /// First frozen onboarding quest id (gather intro).
+ public const string GatherIntroQuestId = "prototype_quest_gather_intro";
+
+ /// First objective id on gather intro step (frozen catalog).
+ public const string GatherIntroFirstObjectiveId = "gather_intro_obj_scrap";
+
+ /// First objective id on operator chain step 0 (frozen catalog).
+ public const string ChainFirstObjectiveId = "chain_obj_gather";
+
/// Keep in sync with scripts/validate_content.py PROTOTYPE_E7M1_CHAIN_TERMINAL_ITEM_ID.
public const string ChainTerminalItemId = "contract_handoff_token";
diff --git a/server/README.md b/server/README.md
index 222d8bb..a4e4b07 100644
--- a/server/README.md
+++ b/server/README.md
@@ -170,7 +170,7 @@ Per-player quest runtime state lives in **`IPlayerQuestStateStore`**, keyed by *
Completed rows cannot regress to active without a reset API (none in prototype). Player ids are normalized (trim + lowercase). Quest ids should be validated via **`IQuestDefinitionRegistry.TryNormalizeKnown`** at operation/HTTP layers (NEO-117+).
-**Storage:** in-memory singleton when **`ConnectionStrings:NeonSprawl`** is unset (seeds configured dev player only). When Postgres is configured, **`PostgresPlayerQuestStateStore`** persists to **`player_quest_progress`** ([`V008__player_quest_progress.sql`](../db/migrations/V008__player_quest_progress.sql)) with **`objective_counters`** JSONB for the current step. Plan: [NEO-116 implementation plan](../../docs/plans/NEO-116-implementation-plan.md).
+**Storage:** in-memory singleton when **`ConnectionStrings:NeonSprawl`** is unset (seeds configured dev player only). When Postgres is configured, **`PostgresPlayerQuestStateStore`** persists to **`player_quest_progress`** ([`V008__player_quest_progress.sql`](../db/migrations/V008__player_quest_progress.sql)) with **`objective_counters`** JSONB for the current step. Plan: [NEO-116 implementation plan](../../docs/plans/NEO-116-implementation-plan.md). Bruno startup smoke: `bruno/neon-sprawl-server/quest-progress/` (health only until E7M1-08 HTTP).
## Encounter definitions (NEO-103)