NEO-63: address code review follow-ups.

AAA depletion test layout, source_kind_not_allowed assertion,
HTTP inventory_full deny test, trim duplicate XP test, doc comments.

docs/reviews/2026-05-24-NEO-63.md

@@ -34,19 +34,19 @@ None.
 
 ## Suggestions
 
-1. **AAA layout in depletion integration test** — `InteractionResourceNodeGatherIntegrationTests.PostInteract_ResourceNode_WhenDepleted_ShouldDenyWithNodeDepletedWithoutGrants` asserts **`Allowed`** inside the Arrange loop (10 pre-gathers). Move loop invariants to **Act** (fire-and-forget or collect last response) and keep all expectations in **Assert**, per [csharp-style](../../.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert) — same pattern flagged on NEO-61.
+1. ~~**AAA layout in depletion integration test** — `InteractionResourceNodeGatherIntegrationTests.PostInteract_ResourceNode_WhenDepleted_ShouldDenyWithNodeDepletedWithoutGrants` asserts **`Allowed`** inside the Arrange loop (10 pre-gathers). Move loop invariants to **Act** (fire-and-forget or collect last response) and keep all expectations in **Assert**, per [csharp-style](../../.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert) — same pattern flagged on NEO-61.~~ **Done.** — pre-gather loop moved to **Act**; **`Assert.All`** on successful responses; depletion + totals asserted in **Assert**.
 
-2. **Tighten skill-deny integration assertion** — `PostInteract_ResourceNode_WhenSalvageDisallowsActivity_ShouldDenyWithoutGrants` only checks `ReasonCode` is non-empty. Assert the expected passthrough **`source_kind_not_allowed`** (from `SkillProgressionSnapshotApi.ReasonSourceKindNotAllowed`) so regressions in XP rollback + reason mapping are caught explicitly.
+2. ~~**Tighten skill-deny integration assertion** — `PostInteract_ResourceNode_WhenSalvageDisallowsActivity_ShouldDenyWithoutGrants` only checks `ReasonCode` is non-empty. Assert the expected passthrough **`source_kind_not_allowed`** (from `SkillProgressionSnapshotApi.ReasonSourceKindNotAllowed`) so regressions in XP rollback + reason mapping are caught explicitly.~~ **Done.**
 
-3. **Optional HTTP `inventory_full` deny** — Engine test covers full-bag deny without depletion; no interact-level test yet. Low priority since **`GatherOperationsTests`** already locks the rule; add one HTTP test if you want parity with depletion deny at the wire boundary.
+3. ~~**Optional HTTP `inventory_full` deny** — Engine test covers full-bag deny without depletion; no interact-level test yet. Low priority since **`GatherOperationsTests`** already locks the rule; add one HTTP test if you want parity with depletion deny at the wire boundary.~~ **Done.** — **`PostInteract_ResourceNode_WhenBagFull_ShouldDenyWithInventoryFullWithoutDepletingNode`**.
 
-4. **Test duplication (optional cleanup)** — `InteractionResourceNodeGatherXpTests` overlaps with `InteractionResourceNodeGatherIntegrationTests` on terminal control and alpha XP smoke. Plan kept both for regression; consider trimming the legacy file to terminal-only once NEO-63 integration tests are trusted.
+4. ~~**Test duplication (optional cleanup)** — `InteractionResourceNodeGatherXpTests` overlaps with `InteractionResourceNodeGatherIntegrationTests` on terminal control and alpha XP smoke. Plan kept both for regression; consider trimming the legacy file to terminal-only once NEO-63 integration tests are trusted.~~ **Done.** — legacy file trimmed to terminal-only; **`InteractionApiTests`** summary points at NEO-63 integration tests.
 
 ## Nits
 
-- Nit: **`SkillProgressionGrantOperations`** summary still references “NEO-41 gather interact hook”; interact now routes through **`GatherOperations`** (NEO-63).
+- ~~Nit: **`SkillProgressionGrantOperations`** summary still references “NEO-41 gather interact hook”; interact now routes through **`GatherOperations`** (NEO-63).~~ **Done.**
 
-- Nit: **`GatherOperations`** class comment says “Interact wiring is NEO-63” — wiring is landed; update to “wired from **`InteractionApi`** (NEO-63)”.
+- ~~Nit: **`GatherOperations`** class comment says “Interact wiring is NEO-63” — wiring is landed; update to “wired from **`InteractionApi`** (NEO-63)”.~~ **Done.**
 
 - Nit: Stacked branch — five commits include NEO-62 plan + engine before NEO-63 wiring. Call out both issue keys in the PR title/description so reviewers know scope.
 

server/NeonSprawl.Server.Tests/Game/Interaction/InteractionApiTests.cs

@@ -7,7 +7,7 @@ using Xunit;
 
 namespace NeonSprawl.Server.Tests.Game.Interaction;
 
-/// <summary>Integration tests for <see cref="InteractionApi"/> (NS-18). NEO-41: <see cref="InteractionResourceNodeGatherXpTests"/> covers gather XP on <c>resource_node</c> kind.</summary>
+/// <summary>Integration tests for <see cref="InteractionApi"/> (NS-18). NEO-63: <see cref="InteractionResourceNodeGatherIntegrationTests"/> covers gather via interact.</summary>
 public class InteractionApiTests
 {
     [Fact]

server/NeonSprawl.Server.Tests/Game/Interaction/InteractionResourceNodeGatherIntegrationTests.cs

@@ -59,27 +59,67 @@ public sealed class InteractionResourceNodeGatherIntegrationTests
         await using var factory = new InMemoryWebApplicationFactory();
         var client = factory.CreateClient();
         await MoveNearAlphaAsync(client);
-        for (var i = 0; i < 10; i++)
+        var request = new InteractionRequest
         {
-            var gather = await client.PostAsJsonAsync(
+            SchemaVersion = InteractionRequest.CurrentSchemaVersion,
-                "/game/players/dev-local-1/interact",
+            InteractableId = PrototypeInteractableRegistry.PrototypeResourceNodeAlphaId,
-                new InteractionRequest
+        };
-                {
+
-                    SchemaVersion = InteractionRequest.CurrentSchemaVersion,
+        // Act — exhaust capacity, then one more interact
-                    InteractableId = PrototypeInteractableRegistry.PrototypeResourceNodeAlphaId,
+        var successfulGathers = new InteractionResponse[10];
-                });
+        for (var i = 0; i < successfulGathers.Length; i++)
-            Assert.Equal(HttpStatusCode.OK, gather.StatusCode);
+        {
-            var body = await gather.Content.ReadFromJsonAsync<InteractionResponse>();
+            var gather = await client.PostAsJsonAsync("/game/players/dev-local-1/interact", request);
-            Assert.NotNull(body);
+            successfulGathers[i] = (await gather.Content.ReadFromJsonAsync<InteractionResponse>())!;
-            Assert.True(body!.Allowed);
         }
 
+        var depletedInteract = await client.PostAsJsonAsync("/game/players/dev-local-1/interact", request);
+        var inventoryAfter = await client.GetAsync("/game/players/dev-local-1/inventory");
+        var progressionAfter = await client.GetAsync("/game/players/dev-local-1/skill-progression");
+
+        // Assert
+        Assert.All(successfulGathers, static r => Assert.True(r.Allowed));
+        Assert.Equal(HttpStatusCode.OK, depletedInteract.StatusCode);
+        var envelope = await depletedInteract.Content.ReadFromJsonAsync<InteractionResponse>();
+        Assert.NotNull(envelope);
+        Assert.False(envelope!.Allowed);
+        Assert.Equal(GatherReasonCodes.NodeDepleted, envelope.ReasonCode);
+        Assert.Equal(10, CountItem(
+            (await inventoryAfter.Content.ReadFromJsonAsync<PlayerInventorySnapshotResponse>())!,
+            "scrap_metal_bulk"));
+        Assert.Equal(
+            10 * GatherSkillXpConstants.ActivityXpPerResourceNodeGather,
+            (await progressionAfter.Content.ReadFromJsonAsync<SkillProgressionSnapshotResponse>())!
+                .Skills!.Single(static s => s.Id == "salvage").Xp);
+    }
+
+    [Fact]
+    public async Task PostInteract_ResourceNode_WhenBagFull_ShouldDenyWithInventoryFullWithoutDepletingNode()
+    {
+        // Arrange
+        await using var factory = new InMemoryWebApplicationFactory();
+        var client = factory.CreateClient();
+        for (var i = 0; i < PlayerInventorySnapshot.BagSlotCount; i++)
+        {
+            await client.PostAsJsonAsync(
+                "/game/players/dev-local-1/inventory",
+                new PlayerInventoryMutationRequest
+                {
+                    SchemaVersion = PlayerInventoryMutationRequest.CurrentSchemaVersion,
+                    MutationKind = "add",
+                    ItemId = "survey_drone_kit",
+                    Quantity = 1,
+                });
+        }
+
+        await MoveNearAlphaAsync(client);
         var inventoryBefore = await client.GetAsync("/game/players/dev-local-1/inventory");
         var progressionBefore = await client.GetAsync("/game/players/dev-local-1/skill-progression");
-        var bagBefore = await inventoryBefore.Content.ReadFromJsonAsync<PlayerInventorySnapshotResponse>();
+        var scrapBefore = CountItem(
-        var skillsBefore = await progressionBefore.Content.ReadFromJsonAsync<SkillProgressionSnapshotResponse>();
+            (await inventoryBefore.Content.ReadFromJsonAsync<PlayerInventorySnapshotResponse>())!,
-        var scrapBefore = CountItem(bagBefore!, "scrap_metal_bulk");
+            "scrap_metal_bulk");
-        var salvageBefore = skillsBefore!.Skills!.Single(static s => s.Id == "salvage").Xp;
+        var salvageBefore = (await progressionBefore.Content.ReadFromJsonAsync<SkillProgressionSnapshotResponse>())!
+            .Skills!.Single(static s => s.Id == "salvage").Xp;
 
         // Act
         var interact = await client.PostAsJsonAsync(
@@ -89,22 +129,24 @@ public sealed class InteractionResourceNodeGatherIntegrationTests
                 SchemaVersion = InteractionRequest.CurrentSchemaVersion,
                 InteractableId = PrototypeInteractableRegistry.PrototypeResourceNodeAlphaId,
             });
-        var inventoryAfter = await client.GetAsync("/game/players/dev-local-1/inventory");
-        var progressionAfter = await client.GetAsync("/game/players/dev-local-1/skill-progression");
 
         // Assert
         Assert.Equal(HttpStatusCode.OK, interact.StatusCode);
         var envelope = await interact.Content.ReadFromJsonAsync<InteractionResponse>();
         Assert.NotNull(envelope);
         Assert.False(envelope!.Allowed);
-        Assert.Equal(GatherReasonCodes.NodeDepleted, envelope.ReasonCode);
+        Assert.Equal(GatherReasonCodes.InventoryFull, envelope.ReasonCode);
-
+        Assert.Equal(
-        var bagAfter = await inventoryAfter.Content.ReadFromJsonAsync<PlayerInventorySnapshotResponse>();
+            scrapBefore,
-        var skillsAfter = await progressionAfter.Content.ReadFromJsonAsync<SkillProgressionSnapshotResponse>();
+            CountItem(
-        Assert.Equal(scrapBefore, CountItem(bagAfter!, "scrap_metal_bulk"));
+                (await (await client.GetAsync("/game/players/dev-local-1/inventory")).Content
+                    .ReadFromJsonAsync<PlayerInventorySnapshotResponse>())!,
+                "scrap_metal_bulk"));
         Assert.Equal(
             salvageBefore,
-            skillsAfter!.Skills!.Single(static s => s.Id == "salvage").Xp);
+            (await (await client.GetAsync("/game/players/dev-local-1/skill-progression")).Content
+                .ReadFromJsonAsync<SkillProgressionSnapshotResponse>())!
+                .Skills!.Single(static s => s.Id == "salvage").Xp);
     }
 
     [Fact]
@@ -171,7 +213,7 @@ public sealed class InteractionResourceNodeGatherIntegrationTests
         var envelope = await interact.Content.ReadFromJsonAsync<InteractionResponse>();
         Assert.NotNull(envelope);
         Assert.False(envelope!.Allowed);
-        Assert.False(string.IsNullOrEmpty(envelope.ReasonCode));
+        Assert.Equal(SkillProgressionSnapshotApi.ReasonSourceKindNotAllowed, envelope.ReasonCode);
 
         var bag = await inventory.Content.ReadFromJsonAsync<PlayerInventorySnapshotResponse>();
         Assert.NotNull(bag);

server/NeonSprawl.Server.Tests/Game/Interaction/InteractionResourceNodeGatherXpTests.cs

@@ -9,7 +9,7 @@ using Xunit;
 
 namespace NeonSprawl.Server.Tests.Game.Interaction;
 
-/// <summary>NEO-41 legacy coverage retained for terminal control; gather side effects covered by NEO-63 integration tests.</summary>
+/// <summary>NEO-41 terminal-only regression; gather side effects covered by NEO-63 integration tests.</summary>
 public sealed class InteractionResourceNodeGatherXpTests
 {
     [Fact]
@@ -43,40 +43,4 @@ public sealed class InteractionResourceNodeGatherXpTests
         var salvage = body!.Skills!.Single(static s => s.Id == "salvage");
         Assert.Equal(0, salvage.Xp);
     }
-
-    [Fact]
-    public async Task PostInteract_ResourceNode_WhenInRange_ShouldGrantSalvageActivityXp_ViaProgressionSnapshot()
-    {
-        // Arrange — node at (12, -6), radius 3; stand at (10, -6) → horizontal distance 2
-        await using var factory = new InMemoryWebApplicationFactory();
-        var client = factory.CreateClient();
-        var move = new MoveCommandRequest
-        {
-            SchemaVersion = MoveCommandRequest.CurrentSchemaVersion,
-            Target = new PositionVector { X = 10, Y = 0.9, Z = -6 },
-        };
-        await client.PostAsJsonAsync("/game/players/dev-local-1/move", move);
-
-        // Act
-        var interact = await client.PostAsJsonAsync(
-            "/game/players/dev-local-1/interact",
-            new InteractionRequest
-            {
-                SchemaVersion = InteractionRequest.CurrentSchemaVersion,
-                InteractableId = PrototypeInteractableRegistry.PrototypeResourceNodeAlphaId,
-            });
-        var snapshot = await client.GetAsync("/game/players/dev-local-1/skill-progression");
-
-        // Assert
-        Assert.Equal(HttpStatusCode.OK, interact.StatusCode);
-        var envelope = await interact.Content.ReadFromJsonAsync<InteractionResponse>();
-        Assert.NotNull(envelope);
-        Assert.True(envelope!.Allowed);
-
-        Assert.Equal(HttpStatusCode.OK, snapshot.StatusCode);
-        var body = await snapshot.Content.ReadFromJsonAsync<SkillProgressionSnapshotResponse>();
-        Assert.NotNull(body);
-        var salvage = body!.Skills!.Single(static s => s.Id == "salvage");
-        Assert.Equal(GatherSkillXpConstants.ActivityXpPerResourceNodeGather, salvage.Xp);
-    }
 }

server/NeonSprawl.Server/Game/Gathering/GatherOperations.cs

@@ -6,7 +6,7 @@ namespace NeonSprawl.Server.Game.Gathering;
 
 /// <summary>
 /// Orchestrates gather side effects: capacity pre-check, inventory grant, skill XP, depletion commit (NEO-62).
-/// Interact wiring is NEO-63.
+/// Wired from <see cref="NeonSprawl.Server.Game.Interaction.InteractionApi"/> on <c>resource_node</c> interact (NEO-63).
 /// </summary>
 public static class GatherOperations
 {

server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantOperations.cs

@@ -2,7 +2,7 @@ using NeonSprawl.Server.Game.Mastery;
 
 namespace NeonSprawl.Server.Game.Skills;
 
-/// <summary>Shared NEO-38 skill XP grant apply (used by HTTP POST and NEO-41 gather interact hook).</summary>
+/// <summary>Shared NEO-38 skill XP grant apply (HTTP POST, gather engine via NEO-63 interact, and other callers).</summary>
 public static class SkillProgressionGrantOperations
 {
     /// <summary>