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

171 lines
11 KiB
Markdown

# NEO-120 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEO-120 |
| **Title** | E7M1-09: POST /game/players/{id}/quests/{questId}/accept |
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-120/e7m1-09-post-gameplayersidquestsquestidaccept |
| **Module** | [E7.M1 — QuestStateMachine](../decomposition/modules/E7_M1_QuestStateMachine.md) · Epic 7 Slice 1 · backlog **E7M1-09** |
| **Branch** | `NEO-120-quest-accept-post` |
| **Precursor** | [NEO-117](https://linear.app/neon-sprawl/issue/NEO-117) — `QuestStateOperations.TryAccept` (**landed on `main`**); [NEO-119](https://linear.app/neon-sprawl/issue/NEO-119) — GET quest-progress (**landed on `main`**) |
| **Pattern** | [NEO-117](NEO-117-implementation-plan.md) — operations + reason codes; [NEO-119](NEO-119-implementation-plan.md) — quest progress row JSON; [NEO-55](NEO-55-implementation-plan.md) / `PlayerInventoryApi` — mutation envelope (`applied` + `reasonCode` + nested snapshot, HTTP **200** on deny) |
| **Blocks** | [NEO-122](https://linear.app/neon-sprawl/issue/NEO-122) — client quest progress + accept HUD (E7M1-11) |
| **Client counterpart** | [NEO-122](https://linear.app/neon-sprawl/issue/NEO-122) — accept binding + feedback label polls this POST then GET quest-progress. Bruno-only accept verification is **not** prototype-complete per [full-stack epic decomposition](../../.cursor/rules/full-stack-epic-decomposition.md). |
## Kickoff clarifications
**No clarifications needed.** [E7M1-09](E7M1-prototype-backlog.md#e7m1-09--post-gameplayersidquestsquestidaccept) backlog, Linear AC, landed [NEO-117](NEO-117-implementation-plan.md) operations, and mutation HTTP precedents (`PlayerInventoryApi`, `PlayerCraftApi`, `AbilityCastApi`) settle scope:
- **Response envelope:** `accepted` + optional `reasonCode` + optional single **`quest`** row (`QuestProgressRowJson` shape from NEO-119) — backlog “updated progress row or deny payload”; inventory-style **200** on structured deny.
- **Request body:** optional v1 — omit body, empty body, or `{ "schemaVersion": 1 }` all valid; **400** only when body is present with wrong `schemaVersion`.
- **Player gate:** `IPositionStateStore`**404** for unknown/blank ids (GET quest-progress / inventory precedent) before `TryAccept`.
- **Unknown quest:** **200** + `accepted: false` + `reasonCode: unknown_quest` (operations layer; not HTTP 404).
- **Deny with snapshot:** `already_active` / `already_completed` include current **`quest`** row from operations snapshot (no mutation on deny).
- **Manual QA doc:** skip `docs/manual-qa/NEO-120.md` — server-only; Bruno + API integration tests (NEO-119 precedent).
## Goal, scope, and out-of-scope
**Goal:** Expose player-initiated quest accept over HTTP so Godot (and Bruno) can transition a quest from **`not_started``active`** with structured deny feedback when prerequisites or state block accept.
**In scope (from Linear + [E7M1-09](E7M1-prototype-backlog.md#e7m1-09--post-gameplayersidquestsquestidaccept)):**
- **`POST /game/players/{id}/quests/{questId}/accept`** — `QuestAcceptApi`; optional request body v1; response with **`accepted`**, optional **`reasonCode`**, optional **`quest`** progress row.
- Wire **`QuestStateOperations.TryAccept`** (inventory refresh + step completion on accept already in operations).
- Shared row projection with GET (`QuestProgressRowJson` from `QuestStepState` / missing row).
- Bruno `bruno/neon-sprawl-server/quest-progress/` — happy accept, prerequisite deny, duplicate accept.
- Integration tests (AAA): 404 player gate; happy accept; `prerequisite_incomplete`; `already_active`; `unknown_quest`.
- `server/README.md` accept route section.
**Out of scope (from Linear + backlog):**
- Abandon/reset POST.
- Godot HUD ([NEO-122](https://linear.app/neon-sprawl/issue/NEO-122)).
- Telemetry hook comments ([NEO-121](https://linear.app/neon-sprawl/issue/NEO-121)).
- Dev fixture reset API for quest rows.
- `docs/manual-qa/NEO-120.md` (server-only per kickoff).
## Acceptance criteria checklist
- [x] Successful accept transitions row **`not_started``active`**.
- [x] Deny returns structured **`reasonCode`** without mutation.
- [x] Bruno + integration tests pass in CI.
## Implementation reconciliation (shipped)
- **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:** 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.
## Technical approach
1. **Route:** **`POST /game/players/{id}/quests/{questId}/accept`** — inject **`IPositionStateStore`**, **`IQuestDefinitionRegistry`**, **`IPlayerQuestStateStore`**, **`IPlayerInventoryStore`**, **`IItemDefinitionRegistry`**, **`TimeProvider`**.
2. **Request:** `QuestAcceptRequest` with `schemaVersion` **1**. Handler accepts **`null`**, empty JSON, or `{ "schemaVersion": 1 }`. **400** when body is non-null and `schemaVersion` ≠ 1.
3. **Player gate:** Trim `id`; return **404** when empty or position missing (mirror **`QuestProgressApi`** GET).
4. **Accept:** `QuestStateOperations.TryAccept(trimmedId, questId, …)``questId` from route (operations normalizes via registry).
5. **Response mapping** (`QuestAcceptResponse` v1):
| Outcome | HTTP | `accepted` | `reasonCode` | `quest` row |
|---------|------|------------|--------------|-------------|
| Success | 200 | `true` | omitted | active row from snapshot |
| Deny (no snapshot) | 200 | `false` | operations code | omitted |
| Deny (with snapshot) | 200 | `false` | operations code | row from snapshot (`already_active`, `already_completed`) |
Map `QuestStepState``QuestProgressRowJson` via shared helper extracted from **`QuestProgressApi.MapRow`** (e.g. **`MapQuestProgressRow`**) so GET and POST stay aligned.
6. **Register:** `app.MapQuestAcceptApi()` in **`Program.cs`** immediately after **`MapQuestProgressApi()`**.
7. **Bruno** (`bruno/neon-sprawl-server/quest-progress/`):
- **`Accept gather intro.bru`** — POST `dev-local-1` / `prototype_quest_gather_intro` → 200, `accepted: true`, `quest.status === active`, `currentStepIndex === 0`.
- **`Accept refine prerequisite deny.bru`** — POST refine intro without gather complete → 200, `accepted: false`, `reasonCode === prerequisite_incomplete`, no active refine row on subsequent GET.
- **`Accept duplicate.bru`** — accept gather twice → second 200, `accepted: false`, `reasonCode === already_active`, `quest.status === active`.
8. **Docs:** Add **`server/README.md`** POST accept section (curl, response shape, reason codes). Update E7.M1 module snapshot / alignment register / E7M1 backlog on story land.
### Expected v1 success response (prototype)
```json
{
"schemaVersion": 1,
"accepted": true,
"quest": {
"questId": "prototype_quest_gather_intro",
"status": "active",
"currentStepIndex": 0,
"objectiveCounters": {}
}
}
```
### Expected v1 deny response (prerequisite)
```json
{
"schemaVersion": 1,
"accepted": false,
"reasonCode": "prerequisite_incomplete"
}
```
### Accept flow
```mermaid
flowchart TD
R[POST accept] --> B{Body schema ok?}
B -->|no| B400[400]
B -->|yes| G{Known player?}
G -->|no| N404[404]
G -->|yes| A[QuestStateOperations.TryAccept]
A --> M[Map QuestAcceptResponse]
M --> J[JSON 200]
```
## Files to add
| Path | Purpose |
|------|---------|
| `server/NeonSprawl.Server/Game/Quests/QuestAcceptApi.cs` | `Map*` extension; player gate; `TryAccept`; response mapping. |
| `server/NeonSprawl.Server/Game/Quests/QuestAcceptDtos.cs` | Optional request + v1 accept response DTOs. |
| `server/NeonSprawl.Server.Tests/Game/Quests/QuestAcceptApiTests.cs` | AAA HTTP integration: 404, happy accept, prerequisite deny, duplicate accept, unknown quest. |
| `bruno/neon-sprawl-server/quest-progress/Accept gather intro.bru` | Bruno happy-path accept. |
| `bruno/neon-sprawl-server/quest-progress/Accept refine prerequisite deny.bru` | Bruno prerequisite deny. |
| `bruno/neon-sprawl-server/quest-progress/Accept duplicate.bru` | Bruno duplicate accept deny. |
| `docs/plans/NEO-120-implementation-plan.md` | This plan. |
## Files to modify
| Path | Rationale |
|------|-----------|
| `server/NeonSprawl.Server/Game/Quests/QuestProgressApi.cs` | Extract shared `MapQuestProgressRow` (or equivalent) for GET + accept POST row projection. |
| `server/NeonSprawl.Server/Program.cs` | Register `MapQuestAcceptApi()` after `MapQuestProgressApi()`. |
| `server/README.md` | Document POST accept route, request/response shape, reason codes; replace E7M1-09 deferral stub. |
| `docs/decomposition/modules/E7_M1_QuestStateMachine.md` | HTTP quest accept bullet (on story complete). |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E7.M1 row — NEO-120 HTTP (on story complete). |
| `docs/decomposition/modules/module_dependency_register.md` | E7.M1 note — NEO-120 (on story complete). |
| `docs/plans/E7M1-prototype-backlog.md` | E7M1-09 acceptance checkboxes (on story complete). |
## Tests
| 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. **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
| Question / risk | Agent recommendation | Status |
|-----------------|----------------------|--------|
| **Response field `accepted` vs `applied`** | Use **`accepted`** — matches `AbilityCastResponse` / perk `selected` mutation naming; backlog does not mandate `applied` | **adopted** |
| **Shared row mapper location** | Extract from `QuestProgressApi` to internal static helper in same file or small shared type — avoid duplicating status/counter mapping | **adopted** |
| **Accept may auto-complete first step** | Operations already run inventory wiring + step completion on accept (NEO-118); tests assert post-accept row as returned, not forced step 0 if wiring advances | **adopted** |
| **Bruno collection ordering** | Add accept `.bru` files beside existing quest-progress folder; no new collection root | **adopted** |