# NEO-41 — Implementation plan

## Story reference

| Field | Value |
|--------|--------|
| **Key** | NEO-41 |
| **Title** | Gather loop awards skill XP (sourceKind: activity) |
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-41/gather-loop-awards-skill-xp-sourcekind-activity |
| **Labels** | E3.M1, E2.M2, server |
| **Depends on** | [NEO-38](https://linear.app/neon-sprawl/issue/NEO-38) — **Done** (authoritative `POST …/skill-progression`). |

## Kickoff clarifications

| Topic | Question / note | Resolution |
|--------|-----------------|------------|
| **What counts as “gather completion”?** | Repo has no `GatherResult` / E3.M1 gather API yet; `InteractionApi` + prototype **`resource_node`** interactable exist. | **User (AskQuestion):** **Prototype stand-in** — treat a **successful** `POST /game/players/{id}/interact` whose resolved interactable has **`Kind == "resource_node"`** (today: `PrototypeInteractableRegistry.PrototypeResourceNodeAlphaId` / `prototype_resource_node_alpha`) as **gather complete** for this slice. |
| **XP amount** | Linear does not fix per-gather XP. | **Plan default:** **10** XP per qualifying interact (prototype constant, server-side only), same order of magnitude as NEO-38 examples; adjust in implementation PR if product wants a different number before merge. |
| **Target skill** | Which `SkillDef.id` receives the grant? | **`salvage`** — prototype **gather** anchor per [E2_M1_SkillDefinitionRegistry.md](../decomposition/modules/E2_M1_SkillDefinitionRegistry.md); `allowedXpSourceKinds` already includes **`activity`**. |

## Goal, scope, and out-of-scope

**Goal:** When a **gather-class** prototype interaction succeeds, apply **skill XP** through the **same authoritative persistence and validation rules** as NEO-38 (`ISkillDefinitionRegistry` allowlist, `IPlayerSkillProgressionStore`, `ISkillLevelCurve`), with **`sourceKind: activity`**. **No** ad-hoc client XP counters.

**In scope (from Linear):**

- Gather completion → skill XP via the **same path as NEO-38** (shared server logic, not a duplicate client-only bump).
- **Never** emit a grant when **`activity`** is not allowed for the chosen skill (guard + unit/integration coverage).
- **Testable** (and manually verifiable) progression: **`salvage`** XP increases after a successful **`resource_node`** interact; other interactables unchanged.

**Out of scope:**

- Full E3.M1 **`GatherResult`** / yield tables / inventory ([E3_M1_ResourceNodeAndGatherLoop.md](../decomposition/modules/E3_M1_ResourceNodeAndGatherLoop.md) backlog beyond this wire).
- **Content-driven** XP amounts or multi-skill-per-node mapping (follow-up once gather content exists).
- Changing **`InteractionResponse`** schema for XP echo (visibility via **`GET …/skill-progression`** after interact is enough for this slice).

## Acceptance criteria checklist

- [x] Gather completion grants skill XP through the **same authoritative path** as NEO-38 (shared grant application; no ad-hoc client XP).
- [x] Disallowed `sourceKind` / skill combinations **never** emitted from gather (defensive: only `salvage` + `activity` for `resource_node` kind; registry check before apply).
- [x] Visible or **testable** progression: automated test (and manual QA) shows **`salvage`** XP increases after successful **`resource_node`** interact from in-range position.

## Technical approach

1. **Extract** NEO-38 grant apply logic from `SkillProgressionSnapshotApi` into a **single shared helper or small service** in `Game/Skills/` (e.g. static `SkillProgressionGrantOperations.ApplyGrant` or `ISkillProgressionGrantApplier`) that:
   - Accepts `playerId`, `skillId`, `amount`, `sourceKind`, and dependencies (`ISkillDefinitionRegistry`, `IPlayerSkillProgressionStore`, `ISkillLevelCurve`).
   - Performs the same validation order as today: positive amount → known skill → `sourceKind` in `AllowedXpSourceKinds` → `TryApplyXpDelta` → level curve → `SkillProgressionGrantResponse` (success or structured deny with snapshot).
   - Does **not** re-check `IPositionStateStore` for POST callers that already gated; interact path must only run after existing interact **Allowed** path (player already known to exist at validated position).

2. **`SkillProgressionSnapshotApi` POST** — delegate body handling to the shared helper so HTTP contract stays identical.

3. **`InteractionApi` POST** — after `Allowed == true`, if `entry.Kind == "resource_node"` (string compare **ordinal** against literal `"resource_node"` or a shared constant on `PrototypeInteractableRegistry`), call the helper with **`salvage`**, fixed **`GatherActivityXpAmount` (10)**, **`activity`**. On deny from helper, **do not** change interact outcome: interaction remains **Allowed**; XP grant is a **silent no-op** when validation fails (unexpected if catalog stable) — **prefer**: if deny, still return same `InteractionResponse` as today (gather “succeeds” UX) but **no** XP applied; add **unit test** that corrupt catalog case does not throw. *Clarification:* if `TryApplyXpDelta` returns false (e.g. Postgres missing row), same — no XP, interaction still allowed.

4. **Constants** — `PrototypeInteractableRegistry.ResourceNodeKind` or document `Kind` string `"resource_node"` as the gather trigger; **`GatherActivityXpAmount = 10`** in one place (e.g. `GatherSkillXpConstants.cs`).

5. **Telemetry** — NEO-40 hook sites on `POST …/skill-progression` still fire when clients use that API; **interact-triggered** grants should go through the **same helper** so persistence matches; optional follow-up to duplicate hook comments on interact path — **out of scope** unless trivial one-line reference in helper.

6. **Docs / Bruno / README** — Describe “prototype gather = interact `prototype_resource_node_alpha` in range → `salvage` XP” and curl order: move → interact → GET progression.

## Files to add

| Path | Purpose |
|------|---------|
| `server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantOperations.cs` | Shared NEO-38 grant apply + snapshot envelope for POST and gather hook. |
| `server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantApplyOutcome.cs` | `TryApplyGrant` outcome kind + optional response (deny / granted / store missing). |
| `server/NeonSprawl.Server/Game/Skills/GatherSkillXpConstants.cs` | Prototype `salvage` + `activity` + XP amount for resource-node gather. |
| `server/NeonSprawl.Server.Tests/Game/Interaction/InteractionResourceNodeGatherXpTests.cs` | In-range `resource_node` interact increases `salvage` XP on GET snapshot; terminal interact does not; stub-registry deny path. |
| `server/NeonSprawl.Server.Tests/Game/Interaction/SalvageActivityDeniedRegistryWebApplicationFactory.cs` | Test host mirroring in-memory overrides with **`salvage`** excluding **`activity`** for defensive coverage. |
| `docs/manual-qa/NEO-41.md` | curl: move near node → interact → GET skill-progression. |
| `bruno/neon-sprawl-server/position/Post move near prototype resource node.bru` | Setup move for NEO-41 Bruno flow. |
| `bruno/neon-sprawl-server/interaction/Post interact prototype resource node gather.bru` | Bruno: interact `prototype_resource_node_alpha` after move. |

## Files to modify

| Path | Rationale |
|------|-----------|
| `server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs` | Call shared grant helper from POST handler; remove inlined duplicate logic. |
| `server/NeonSprawl.Server/Game/Interaction/InteractionApi.cs` | After successful interact on `resource_node` kind, invoke grant helper with `salvage` / `activity` / fixed amount; extend minimal API lambda parameter list for new services. |
| `server/README.md` | Short subsection: prototype gather XP via interact + link to NEO-41 manual QA. |
| `docs/decomposition/modules/E3_M1_ResourceNodeAndGatherLoop.md` | One-sentence **implementation anchor**: prototype gather completion currently signaled via `InteractionApi` + `Kind: resource_node` until `GatherResult` exists. |

## Tests

| Path | Coverage |
|------|----------|
| `server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantApiTests.cs` | Regression: existing POST cases still pass after refactor to shared helper. |
| New or extended **`Interaction*Tests.cs`** | `resource_node` interact from seeded/in-range position → GET `…/skill-progression` shows increased **`salvage`** XP; **`prototype_terminal`** interact → **`salvage`** XP unchanged. |
| **`SalvageActivityDeniedRegistryWebApplicationFactory`** + third test method | Stub **`ISkillDefinitionRegistry`** where **`salvage`** disallows **`activity`**: interact still **`allowed: true`**, **`salvage`** XP stays **0** (no throw). |

## Open questions / risks

- **Long-term:** Real E3.M1 **`GatherResult`** should become the single completion signal; this interact hook is a **deliberate prototype stand-in** — migrate grant call to gather pipeline without changing NEO-38 POST contract.
- **Risk:** Players could spam interact for XP; **out of scope** for NEO-41 (cooldowns / depletion in E3.M1).
