NEO-120: Address code review test coverage gaps.

Add HTTP tests for already_completed deny snapshot and empty {} body;
tighten already_active AAA arrange. Strike through resolved review items.

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

@@ -57,7 +57,7 @@
 - **Route:** `POST /game/players/{id}/quests/{questId}/accept` — `QuestAcceptApi.MapResponse`; 404 via `IPositionStateStore`; optional body v1.
 - **DTOs:** `QuestAcceptRequest`, `QuestAcceptResponse` (`accepted`, `reasonCode`, `quest` row).
 - **Shared mapper:** `QuestProgressApi.MapQuestProgressRow` overloads for GET + accept POST.
-- **Tests:** eight AAA cases in `QuestAcceptApiTests`.
+- **Tests:** ten AAA cases in `QuestAcceptApiTests` (incl. `already_completed` deny snapshot, empty `{}` body).
 - **Bruno:** accept gather intro, refine prerequisite deny, duplicate accept.
 - **Docs:** `server/README.md`; E7.M1 module snapshot + backlog updated.
 
@@ -157,7 +157,7 @@ flowchart TD
 
 | Test file | What it covers |
 |-----------|----------------|
-| `QuestAcceptApiTests.cs` | **404:** unknown player + whitespace id. **Happy accept:** POST gather intro → `accepted: true`, `quest.status` active, step 0, empty counters; GET confirms active. **Prerequisite deny:** POST refine intro without gather complete → `prerequisite_incomplete`, GET still `not_started` for refine. **Duplicate accept:** second POST → `already_active` with `quest` snapshot, no counter drift. **Unknown quest:** bogus quest id → `unknown_quest`. **Optional body:** POST with no body succeeds for happy accept. |
+| `QuestAcceptApiTests.cs` | **404:** unknown player + whitespace id. **Happy accept:** POST gather intro → `accepted: true`, `quest.status` active, step 0, empty counters; GET confirms active. **Prerequisite deny:** POST refine intro without gather complete → `prerequisite_incomplete`, GET still `not_started` for refine. **Duplicate accept:** second POST → `already_active` with `quest` snapshot. **Completed deny:** accept + complete gather intro → re-accept → `already_completed` with `quest.status` completed + `completedAt`. **Unknown quest:** bogus quest id → `unknown_quest`. **Optional body:** POST with no body or `{}` succeeds for happy accept. |
 | Existing quest test suite | Regression smoke — operations + GET unchanged when accept not called. |
 
 ## Open questions / risks

docs/reviews/2026-06-07-NEO-120.md

@@ -32,15 +32,15 @@ NEO-120 adds **`POST /game/players/{id}/quests/{questId}/accept`** — a thin HT
 
 ## Suggestions
 
-1. **HTTP test for `already_completed` deny with snapshot** — The plan response table and README document **`already_completed`** returning **`accepted: false`** plus a **`quest`** row. Integration tests cover **`already_active`** and operations-layer **`TryAccept_ShouldDenyAlreadyCompleted`** exists in `QuestStateOperationsTests`, but no HTTP test exercises **`MapResponse`** for a completed row (status **`completed`**, optional **`completedAt`**). A single test completing gather intro then re-accepting would lock the wire contract for NEO-122.
+1. ~~**HTTP test for `already_completed` deny with snapshot** — The plan response table and README document **`already_completed`** returning **`accepted: false`** plus a **`quest`** row. Integration tests cover **`already_active`** and operations-layer **`TryAccept_ShouldDenyAlreadyCompleted`** exists in `QuestStateOperationsTests`, but no HTTP test exercises **`MapResponse`** for a completed row (status **`completed`**, optional **`completedAt`**). A single test completing gather intro then re-accepting would lock the wire contract for NEO-122.~~ **Done.** `PostQuestAccept_ShouldDenyAlreadyCompleted_WhenQuestAlreadyCompleted` in `QuestAcceptApiTests.cs`.
 
-2. **Empty `{}` body test** — README documents `{}` as valid (default **`schemaVersion` 0**). Tests cover omitted body and explicit **`schemaVersion: 1`**; a one-liner POST with **`StringContent("{}")`** would close the loop on the documented third valid shape.
+2. ~~**Empty `{}` body test** — README documents `{}` as valid (default **`schemaVersion` 0**). Tests cover omitted body and explicit **`schemaVersion: 1`**; a one-liner POST with **`StringContent("{}")`** would close the loop on the documented third valid shape.~~ **Done.** `PostQuestAccept_ShouldReturnAcceptedTrue_WhenRequestBodyIsEmptyObject` in `QuestAcceptApiTests.cs`.
 
 ## Nits
 
 - Nit: **`MapQuestProgressRow(QuestStepState)`** throws **`InvalidOperationException`** on unknown **`QuestProgressStatus`** (would surface as 500). Inherited defensive guard from NEO-119; acceptable for prototype.
 
-- Nit: **`PostQuestAccept_ShouldDenyAlreadyActive_WhenQuestAlreadyAccepted`** asserts on the first accept response inside **Arrange** (lines 159–162). Works fine; optional move of **`firstBody`** checks into **Assert** if strict AAA phase purity is desired.
+- ~~Nit: **`PostQuestAccept_ShouldDenyAlreadyActive_WhenQuestAlreadyAccepted`** asserts on the first accept response inside **Arrange** (lines 159–162). Works fine; optional move of **`firstBody`** checks into **Assert** if strict AAA phase purity is desired.~~ **Done.** Arrange seeds active row without assertions; assert uses step index **0**.
 
 - Nit: Bruno **`Accept duplicate.bru`** pre-request re-accepts gather intro (idempotent when seq 3 ran). Correct for standalone runs; collection order matters only when hitting a persistent dev server without reset.
 
@@ -62,6 +62,6 @@ dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj
 
 **Bruno:** run the three accept requests in `bruno/neon-sprawl-server/quest-progress/` (seq 3–5) against a local server with fresh `dev-local-1`.
 
-**Reviewer note:** All 8 `QuestAcceptApiTests` and 93 quest-namespace tests passed locally during review.
+**Reviewer note:** All 8 `QuestAcceptApiTests` and 93 quest-namespace tests passed locally during review. **Follow-up:** suggestions 1–2 and nit on `already_active` AAA addressed — 10 `QuestAcceptApiTests` pass.
 
 **Manual:** None required beyond Bruno for this server-only story; Godot accept binding verification belongs to NEO-122.

server/NeonSprawl.Server.Tests/Game/Quests/QuestAcceptApiTests.cs

@@ -2,6 +2,8 @@ using System.Net;
 using System.Net.Http.Json;
 using System.Text;
 using System.Text.Json;
+using Microsoft.Extensions.DependencyInjection;
+using NeonSprawl.Server.Game.Items;
 using NeonSprawl.Server.Game.Quests;
 using NeonSprawl.Server.Tests;
 using Xunit;
@@ -100,6 +102,27 @@ public sealed class QuestAcceptApiTests
         Assert.Equal(GatherQuestId, body.Quest!.QuestId);
     }
 
+    [Fact]
+    public async Task PostQuestAccept_ShouldReturnAcceptedTrue_WhenRequestBodyIsEmptyObject()
+    {
+        // Arrange
+        await using var factory = new InMemoryWebApplicationFactory();
+        var client = factory.CreateClient();
+        using var content = new StringContent("{}", Encoding.UTF8, "application/json");
+
+        // Act
+        var response = await client.PostAsync(
+            $"/game/players/{PlayerId}/quests/{GatherQuestId}/accept",
+            content);
+
+        // Assert
+        Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+        var body = await response.Content.ReadFromJsonAsync<QuestAcceptResponse>();
+        Assert.NotNull(body);
+        Assert.True(body!.Accepted);
+        Assert.Equal(GatherQuestId, body.Quest!.QuestId);
+    }
+
     [Fact]
     public async Task PostQuestAccept_ShouldReturnBadRequest_WhenSchemaVersionInvalid()
     {
@@ -153,13 +176,9 @@ public sealed class QuestAcceptApiTests
         // Arrange
         await using var factory = new InMemoryWebApplicationFactory();
         var client = factory.CreateClient();
-        var first = await client.PostAsync(
+        _ = await client.PostAsync(
             $"/game/players/{PlayerId}/quests/{GatherQuestId}/accept",
             JsonContent.Create(new QuestAcceptRequest { SchemaVersion = QuestAcceptRequest.CurrentSchemaVersion }));
-        Assert.Equal(HttpStatusCode.OK, first.StatusCode);
-        var firstBody = await first.Content.ReadFromJsonAsync<QuestAcceptResponse>();
-        Assert.NotNull(firstBody);
-        Assert.True(firstBody!.Accepted);
 
         // Act
         var response = await client.PostAsync(
@@ -174,7 +193,48 @@ public sealed class QuestAcceptApiTests
         Assert.Equal(QuestStateReasonCodes.AlreadyActive, body.ReasonCode);
         Assert.NotNull(body.Quest);
         Assert.Equal(QuestProgressApi.StatusActive, body.Quest!.Status);
-        Assert.Equal(firstBody.Quest!.CurrentStepIndex, body.Quest.CurrentStepIndex);
+        Assert.Equal(0, body.Quest.CurrentStepIndex);
+    }
+
+    [Fact]
+    public async Task PostQuestAccept_ShouldDenyAlreadyCompleted_WhenQuestAlreadyCompleted()
+    {
+        // Arrange
+        await using var factory = new InMemoryWebApplicationFactory();
+        var client = factory.CreateClient();
+        var questRegistry = factory.Services.GetRequiredService<IQuestDefinitionRegistry>();
+        var progressStore = factory.Services.GetRequiredService<IPlayerQuestStateStore>();
+        var timeProvider = TimeProvider.System;
+        Assert.True(QuestStateOperations.TryAccept(
+            PlayerId,
+            GatherQuestId,
+            questRegistry,
+            progressStore,
+            factory.Services.GetRequiredService<IPlayerInventoryStore>(),
+            factory.Services.GetRequiredService<IItemDefinitionRegistry>(),
+            timeProvider).Success);
+        Assert.True(QuestStateOperations.TryMarkComplete(
+            PlayerId,
+            GatherQuestId,
+            questRegistry,
+            progressStore,
+            timeProvider).Success);
+
+        // Act
+        var response = await client.PostAsync(
+            $"/game/players/{PlayerId}/quests/{GatherQuestId}/accept",
+            JsonContent.Create(new QuestAcceptRequest { SchemaVersion = QuestAcceptRequest.CurrentSchemaVersion }));
+
+        // Assert
+        Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+        var body = await response.Content.ReadFromJsonAsync<QuestAcceptResponse>();
+        Assert.NotNull(body);
+        Assert.False(body!.Accepted);
+        Assert.Equal(QuestStateReasonCodes.AlreadyCompleted, body.ReasonCode);
+        Assert.NotNull(body.Quest);
+        Assert.Equal(QuestProgressApi.StatusCompleted, body.Quest!.Status);
+        Assert.Equal(0, body.Quest.CurrentStepIndex);
+        Assert.NotNull(body.Quest.CompletedAt);
     }
 
     [Fact]