NEO-23: lock JSON shape, GET envelope, selectionApplied, soft lock in plan

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

@@ -19,7 +19,7 @@
 
 - **Stub eligible combat targets:** static registry (≥ **2** distinct ids with world anchors + horizontal **lock radius**) so [NEO-24](https://linear.app/neon-sprawl/issue/NEO-24) can assume deterministic cycling without expanding scope here beyond “exists and validates.”
 - **Selection intent:** `POST` with v1 body; **clear lock** when `targetId` is **omitted or JSON `null`** (explicit rule in XML + README). **Trim** non-null ids; **whitespace-only** after trim → **400** (same spirit as empty `interactableId` on NEO-9).
-- **Authoritative read model:** `TargetState` includes **locked target id** (empty when none), **`validity`** derived from **current** `PositionState` vs stub anchor + radius using **`HorizontalReach`** (X/Z only, inclusive boundary — same policy as NEO-9), and **`sequence`** bumped only when the **persisted lock id** changes (aligns with “optional sequence” in the module’s illustrative table).
+- **Authoritative read model:** `TargetState` includes **`lockedTargetId`** (**always** serialized; JSON **`null`** when no lock — never omit the key), **`validity`** derived from **current** `PositionState` vs stub anchor + radius using **`HorizontalReach`** (X/Z only, inclusive boundary — same policy as NEO-9), and **`sequence`** bumped only when the **persisted lock id** changes (aligns with “optional sequence” in the module’s illustrative table).
 - **`GET`** current `TargetState` for the same player id normalization rules as position/interact APIs.
 - **Unit + integration tests** mirroring `InteractionApiTests` patterns; **server README** subsection documenting routes, curl, and `reasonCode` table.
 
@@ -32,7 +32,7 @@
 ## Acceptance criteria checklist
 
 - [ ] **Server source of truth:** Client cannot persist a lock the server rejects; every **`POST`** response includes the **authoritative** `targetState` after evaluation (success or denial).
-- [ ] **Denials:** **`selectionApplied`: `false`** (or equivalent boolean agreed in implementation) ⇒ **required non-empty `reasonCode`**; **HTTP 200** for logical denials; **400** / **404** rules match NEO-9 spirit (unknown player → **404** before body; malformed v1 → **400**).
+- [ ] **Denials:** **`selectionApplied`: `false`** ⇒ **required non-empty `reasonCode`**; **HTTP 200** for logical denials; **400** / **404** rules match NEO-9 spirit (unknown player → **404** before body; malformed v1 → **400**).
 - [ ] **Movement coupling:** After **`POST /move`** or **`POST /move-stream`**, the same **`GET`**/ **`POST`** response reflects updated **`validity`** (e.g. walk out of stub radius ⇒ `out_of_range` on read without requiring a new select).
 - [ ] **Contract names** in README + XML match the **illustrative** E1.M3 table ([Key contracts](../decomposition/modules/E1_M3_InteractionAndTargetingLayer.md#key-contracts)); any renames are visible in this plan and in PR text, not only in code.
 
@@ -44,21 +44,26 @@
 
 3. **Mutable per-player lock** (e.g. `IPlayerTargetLockStore` + `InMemoryPlayerTargetLockStore`): stores **nullable locked target id** + **monotonic `sequence`** incremented **only** when the lock id changes (clear or swap). Thread-safe in-memory singleton for prototype (same lifetime pattern as `InMemoryPositionStateStore`).
 
-4. **`TargetState` DTO (v1 JSON):** `schemaVersion`, `lockedTargetId` (null or absent in JSON when none — pick one style and document), `validity` enum serialized as stable strings (`ok`, `out_of_range`, `invalid_target`, `none` for “no lock”), `sequence` (int). **Validity** is **computed** on each read: no lock ⇒ `none`; unknown id in store (should not happen) ⇒ `invalid_target`; else horizontal distance vs **`lockRadius`**.
+4. **`TargetState` fields (v1 JSON, shared by GET body and nested POST echo):** `schemaVersion`, `playerId` (echo), **`lockedTargetId`** (string or JSON **`null`** — key **always present**), `validity` (`ok`, `out_of_range`, `invalid_target`, `none`), `sequence` (int). **Validity** is **computed** on each read: no lock ⇒ `validity: none` and `lockedTargetId: null`; unknown id in store (should not happen) ⇒ `invalid_target`; else horizontal distance vs **`lockRadius`**.
 
-5. **`TargetSelectRequest` / `TargetSelectResponse` v1:** Request: `schemaVersion`, optional `targetId`. Response: `schemaVersion`, `selectionApplied` (bool), **`targetState`** (always), **`reasonCode`** (required when `selectionApplied` is false, omitted or null when true — mirror `InteractionResponse` / `reasonCode` rules). Successful **clear** ⇒ `selectionApplied: true`, `targetState` shows no lock.
+5. **`TargetSelectRequest` / `TargetSelectResponse` v1:** Request: `schemaVersion`, optional `targetId`. Response: `schemaVersion`, **`selectionApplied`** (bool), **`targetState`** (object matching the GET field set above — always present), **`reasonCode`** (required when `selectionApplied` is `false`, omitted when `true` — same spirit as `InteractionResponse`). Successful **clear** ⇒ `selectionApplied: true`, `targetState` shows `lockedTargetId: null`, `validity: none`.
 
 6. **Routes** (exact paths in README; suggested):  
-   - `GET /game/players/{id}/target` → `TargetState` envelope (or bare `TargetState` if we prefer — **decide in PR** and keep README consistent).  
+   - `GET /game/players/{id}/target` → **envelope** aligned with **`PositionStateResponse`**: top-level `schemaVersion`, `playerId`, plus lock fields (`lockedTargetId`, `validity`, `sequence`) so read and move APIs share the same “version + player echo + payload” pattern.  
    - `POST /game/players/{id}/target/select` → body `TargetSelectRequest`, response `TargetSelectResponse`.
 
 7. **`Program.cs`:** register store + `app.MapTargetingApi()` (static extension parallel to `MapInteractionApi`).
 
 8. **README:** new subsection **Targeting (NEO-23)** with curl, status table, `reasonCode` list (`unknown_target`, `out_of_range`, optional `invalid_target`, …).
 
-**Decisions (pre-implementation)**
+**Decisions (locked — stakeholder 2026-04-18)**
 
-- **Validity vs persistence:** Lock **id** persists until cleared or swapped; **`validity`** reflects live range so GET after move shows `out_of_range` without auto-clearing (matches “soft lock” until NEO-24 reconciliation policy). *If product prefers auto-clear when out of range, update this plan in the PR.*
+| Topic | Choice |
+|--------|--------|
+| **`lockedTargetId` when no lock** | Property **always present**; value JSON **`null`** (never omit the key). |
+| **GET body shape** | **Envelope** like `PositionStateResponse`: `schemaVersion`, `playerId`, then lock fields (easier evolution and parity with position GET). |
+| **POST success flag** | **`selectionApplied`** (distinct from interact’s `allowed` — avoids “permission” ambiguity). |
+| **Out of range vs stored lock** | **Soft lock:** persisted lock **id** unchanged until clear/swap; **`validity`** becomes `out_of_range` when the player moves away until they clear or select again. **No auto-clear** on read or timer in NEO-23. |
 
 ## Files to add
 
@@ -90,5 +95,5 @@ No automated Godot harness for NEO-23; manual verification is optional HTTP clie
 
 ## Open questions / risks
 
-- **Auto-clear on prolonged `out_of_range`:** deferred; current plan keeps id until explicit clear/swap (document for NEO-24 UX).
+- **NEO-24 UX:** document that UI may show **out of range** while **`lockedTargetId`** stays set until an explicit clear or a successful swap (soft lock).
 - **Postgres vs in-memory target store:** prototype lock store is in-memory only for NEO-23; multi-instance deployment would need shared storage — out of scope until infra story exists.