NEO-48: address code review — Bruno deny, API deny tests

Add self-contained branch_already_chosen Bruno request, HTTP deny smoke
for unknown track/tier/branch and null-body 400, folder docs for level_too_low.

bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru

@@ -0,0 +1,72 @@
+meta {
+  name: POST branch select deny branch already chosen
+  type: http
+  seq: 4
+}
+
+docs {
+  NEO-48: self-contained deny — pre-request grants salvage to level 2 and picks scrap_efficiency;
+  main request conflicts with bulk_haul on the same tier. Expects selected false + branch_already_chosen.
+}
+
+script:pre-request {
+  const baseUrl = bru.getVar("baseUrl");
+  const grant = await bru.sendRequest({
+    method: "POST",
+    url: `${baseUrl}/game/players/dev-local-1/skill-progression`,
+    headers: { "Content-Type": "application/json" },
+    data: {
+      schemaVersion: 1,
+      skillId: "salvage",
+      amount: 100,
+      sourceKind: "activity",
+    },
+  });
+  if (grant.status !== 200) {
+    throw new Error(`salvage grant failed: ${grant.status}`);
+  }
+  const firstPick = await bru.sendRequest({
+    method: "POST",
+    url: `${baseUrl}/game/players/dev-local-1/perk-state`,
+    headers: { "Content-Type": "application/json" },
+    data: {
+      schemaVersion: 1,
+      skillId: "salvage",
+      tierIndex: 1,
+      branchId: "scrap_efficiency",
+    },
+  });
+  if (firstPick.status !== 200 || firstPick.data?.selected !== true) {
+    throw new Error(`tier-1 pick failed: ${firstPick.status} ${JSON.stringify(firstPick.data)}`);
+  }
+}
+
+post {
+  url: {{baseUrl}}/game/players/dev-local-1/perk-state
+  body: json
+  auth: none
+}
+
+body:json {
+  {
+    "schemaVersion": 1,
+    "skillId": "salvage",
+    "tierIndex": 1,
+    "branchId": "bulk_haul"
+  }
+}
+
+tests {
+  test("denies second pick with branch_already_chosen", function () {
+    expect(res.getStatus()).to.equal(200);
+    const body = res.getBody();
+    expect(body.schemaVersion).to.equal(1);
+    expect(body.selected).to.equal(false);
+    expect(body.reasonCode).to.equal("branch_already_chosen");
+    expect(body.unlockedEvents).to.be.an("array").that.is.empty;
+    const salvage = body.perkState.branchPicks.find((t) => t.skillId === "salvage");
+    expect(salvage).to.be.an("object");
+    const pick = salvage.picks.find((p) => p.tierIndex === 1);
+    expect(pick.branchId).to.equal("scrap_efficiency");
+  });
+}

bruno/neon-sprawl-server/perk-state/folder.bru

@@ -1,3 +1,9 @@
 meta {
   name: perk-state
 }
+
+docs {
+  NEO-48 perk-state HTTP smoke. Run folder in seq order for full coverage.
+  Post branch select deny level too low: passes if salvage is still level 1; if dev-local-1 already has salvage XP from other runs, restart the server (or reset DB in Postgres mode) before expecting level_too_low.
+  Post branch select deny branch already chosen: self-contained (pre-request grant + first pick); safe to re-run.
+}

docs/manual-qa/NEO-48.md

@@ -44,6 +44,7 @@ curl -sS -X POST "http://localhost:5253/game/players/dev-local-1/perk-state" \
 ## Bruno
 
 - [ ] Run folder `bruno/neon-sprawl-server/perk-state/` (GET + POST happy + deny smoke).
+- [ ] Run **`Post branch select deny branch already chosen`** (self-contained; pre-request grant + first pick).
 
 ## Regression
 

docs/reviews/2026-05-17-NEO-48.md

@@ -2,7 +2,8 @@
 
 **Date:** 2026-05-17  
 **Scope:** Branch `NEO-48-perk-state-get-branch-selection-post-bruno` · commits `5290044`–`39c54c3` vs `main`  
-**Base:** `main`
+**Base:** `main`  
+**Follow-up:** Code-review suggestions 1–3 and nit 1 addressed in a follow-up commit on the same branch.
 
 ## Verdict
 
@@ -31,13 +32,13 @@ None.
 
 ## Suggestions
 
-1. **Bruno deny for `branch_already_chosen`** — Plan and manual QA describe a second POST to the same tier with a different branch; automated tests cover it (`PostPerkState_ShouldDenyBranchAlreadyChosen_OnSecondPick`), but Bruno only ships `Post branch select deny level too low.bru`. A fourth request (grant + first pick + conflicting second pick) would make the collection self-contained for CI/manual runs without relying on manual QA alone.
-2. **Optional thin HTTP deny smoke** — Engine tests lock `unknown_track`, `unknown_tier`, and `unknown_branch`; one API test each (bad `skillId`, bad `tierIndex`, bad `branchId` on dev player after level gate) would guard JSON field mapping if DTOs change. Non-blocking given `PerkUnlockEngineTests` coverage.
-3. **POST null body** — Handlers return **400** for `body is null` (same as skill-progression). Neither API suite asserts empty-body **400** today; add a one-liner in `PerkStateApiTests` if you want parity documentation in tests.
+1. ~~**Bruno deny for `branch_already_chosen`** — Plan and manual QA describe a second POST to the same tier with a different branch; automated tests cover it (`PostPerkState_ShouldDenyBranchAlreadyChosen_OnSecondPick`), but Bruno only ships `Post branch select deny level too low.bru`. A fourth request (grant + first pick + conflicting second pick) would make the collection self-contained for CI/manual runs without relying on manual QA alone.~~ **Done.** Added `Post branch select deny branch already chosen.bru` with `script:pre-request` grant + first pick via `bru.sendRequest`.
+2. ~~**Optional thin HTTP deny smoke** — Engine tests lock `unknown_track`, `unknown_tier`, and `unknown_branch`; one API test each (bad `skillId`, bad `tierIndex`, bad `branchId` on dev player after level gate) would guard JSON field mapping if DTOs change. Non-blocking given `PerkUnlockEngineTests` coverage.~~ **Done.** `PostPerkState_ShouldDenyUnknownTrack_*`, `UnknownTier_*`, `UnknownBranch_*` in `PerkStateApiTests`.
+3. ~~**POST null body** — Handlers return **400** for `body is null` (same as skill-progression). Neither API suite asserts empty-body **400** today; add a one-liner in `PerkStateApiTests` if you want parity documentation in tests.~~ **Done.** `PostPerkState_ShouldReturnBadRequest_WhenBodyNull`.
 
 ## Nits
 
-- Nit: `Post branch select deny level too low.bru` intentionally passes when salvage is already leveled (restart note) — acceptable for shared dev player; document in Bruno folder `docs` if operators hit false greens often.
+- ~~Nit: `Post branch select deny level too low.bru` intentionally passes when salvage is already leveled (restart note) — acceptable for shared dev player; document in Bruno folder `docs` if operators hit false greens often.~~ **Done.** Folder `docs` block in `perk-state/folder.bru`.
 - Nit: Path-auto tier-2 unlocks surfaced in **`unlockedEvents`** use **`source: level_up`** (not `branch_pick`) — consistent with NEO-47/NEO-49 notes; clients/telemetry should treat `source` as unlock channel, not HTTP verb.
 - Nit: `PostPerkState_ShouldSelectTier1_AndGetReflectsPick_AfterSalvageLevel2` issues a follow-up **GET** inside **Assert** for end-to-end verification — valid AAA (verification in Assert); grant status checks in **Arrange** match other API tests in the repo.
 

server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs

@@ -1,6 +1,8 @@
 using System.Linq;
 using System.Net;
+using System.Net.Http;
 using System.Net.Http.Json;
+using System.Text;
 using NeonSprawl.Server.Game.Mastery;
 using NeonSprawl.Server.Game.Skills;
 using Xunit;
@@ -67,6 +69,22 @@ public sealed class PerkStateApiTests
         Assert.Empty(body.UnlockedPerkIds);
     }
 
+    [Fact]
+    public async Task PostPerkState_ShouldReturnBadRequest_WhenBodyNull()
+    {
+        // Arrange
+        await using var factory = new InMemoryWebApplicationFactory();
+        var client = factory.CreateClient();
+
+        // Act
+        var response = await client.PostAsync(
+            $"/game/players/{DevPlayer}/perk-state",
+            new StringContent(string.Empty, Encoding.UTF8, "application/json"));
+
+        // Assert
+        Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
+    }
+
     [Fact]
     public async Task PostPerkState_ShouldReturnBadRequest_WhenSchemaVersionMismatch()
     {
@@ -156,15 +174,84 @@ public sealed class PerkStateApiTests
         Assert.Equal("scrap_efficiency", Assert.Single(getTrack.Picks).BranchId);
     }
 
+    private static async Task GrantSalvageLevel2Async(HttpClient client)
+    {
+        var grant = await client.PostAsJsonAsync(
+            $"/game/players/{DevPlayer}/skill-progression",
+            SalvageGrant(100));
+        Assert.Equal(HttpStatusCode.OK, grant.StatusCode);
+    }
+
+    [Fact]
+    public async Task PostPerkState_ShouldDenyUnknownTrack_WhenSkillIdInvalid()
+    {
+        // Arrange
+        await using var factory = new InMemoryWebApplicationFactory();
+        var client = factory.CreateClient();
+        await GrantSalvageLevel2Async(client);
+
+        // Act
+        var response = await client.PostAsJsonAsync(
+            $"/game/players/{DevPlayer}/perk-state",
+            SelectBranch("refine", 1, "scrap_efficiency"));
+
+        // Assert
+        Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+        var envelope = await response.Content.ReadFromJsonAsync<PerkBranchSelectResponse>();
+        Assert.NotNull(envelope);
+        Assert.False(envelope!.Selected);
+        Assert.Equal(PerkUnlockReasonCodes.UnknownTrack, envelope.ReasonCode);
+    }
+
+    [Fact]
+    public async Task PostPerkState_ShouldDenyUnknownTier_WhenTierIndexInvalid()
+    {
+        // Arrange
+        await using var factory = new InMemoryWebApplicationFactory();
+        var client = factory.CreateClient();
+        await GrantSalvageLevel2Async(client);
+
+        // Act
+        var response = await client.PostAsJsonAsync(
+            $"/game/players/{DevPlayer}/perk-state",
+            SelectBranch("salvage", 99, "scrap_efficiency"));
+
+        // Assert
+        Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+        var envelope = await response.Content.ReadFromJsonAsync<PerkBranchSelectResponse>();
+        Assert.NotNull(envelope);
+        Assert.False(envelope!.Selected);
+        Assert.Equal(PerkUnlockReasonCodes.UnknownTier, envelope.ReasonCode);
+    }
+
+    [Fact]
+    public async Task PostPerkState_ShouldDenyUnknownBranch_WhenBranchIdInvalid()
+    {
+        // Arrange
+        await using var factory = new InMemoryWebApplicationFactory();
+        var client = factory.CreateClient();
+        await GrantSalvageLevel2Async(client);
+
+        // Act
+        var response = await client.PostAsJsonAsync(
+            $"/game/players/{DevPlayer}/perk-state",
+            SelectBranch("salvage", 1, "not_a_branch"));
+
+        // Assert
+        Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+        var envelope = await response.Content.ReadFromJsonAsync<PerkBranchSelectResponse>();
+        Assert.NotNull(envelope);
+        Assert.False(envelope!.Selected);
+        Assert.Equal(PerkUnlockReasonCodes.UnknownBranch, envelope.ReasonCode);
+    }
+
     [Fact]
     public async Task PostPerkState_ShouldDenyBranchAlreadyChosen_OnSecondPick()
     {
         // Arrange
         await using var factory = new InMemoryWebApplicationFactory();
         var client = factory.CreateClient();
-        Assert.Equal(HttpStatusCode.OK, (await client.PostAsJsonAsync(
-            $"/game/players/{DevPlayer}/skill-progression",
-            SalvageGrant(100))).StatusCode);
+        await GrantSalvageLevel2Async(client);
         Assert.Equal(HttpStatusCode.OK, (await client.PostAsJsonAsync(
             $"/game/players/{DevPlayer}/perk-state",
             SelectBranch("salvage", 1, "scrap_efficiency"))).StatusCode);