From 8d9d02fde56670f3441adc81e0c66d9fccade387 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 27 Jun 2026 21:43:46 -0400 Subject: [PATCH 1/7] NEO-149: add E7M4-06 implementation plan for contract completion wiring Kickoff plan for ContractCompletionOperations orchestrator, encounter hook after E5.M3 loot, and integration test coverage; Bruno HTTP loop deferred to NEO-151. --- docs/plans/NEO-149-implementation-plan.md | 170 ++++++++++++++++++++++ 1 file changed, 170 insertions(+) create mode 100644 docs/plans/NEO-149-implementation-plan.md diff --git a/docs/plans/NEO-149-implementation-plan.md b/docs/plans/NEO-149-implementation-plan.md new file mode 100644 index 0000000..aef0a3a --- /dev/null +++ b/docs/plans/NEO-149-implementation-plan.md @@ -0,0 +1,170 @@ +# NEO-149 — E7M4-06: ContractCompletionOperations + encounter wiring + +**Linear:** [NEO-149](https://linear.app/neon-sprawl/issue/NEO-149) +**Branch:** `NEO-149-e7m4-06-contract-completion-encounter-wiring` +**Backlog:** [E7M4-pre-production-backlog.md](E7M4-pre-production-backlog.md) — **E7M4-06** +**Module:** [E7_M4_ContractMissionGenerator.md](../decomposition/modules/E7_M4_ContractMissionGenerator.md) +**Pattern:** [NEO-128-implementation-plan.md](NEO-128-implementation-plan.md) — deliver-then-mark + idempotent replay; [NEO-147-implementation-plan.md](NEO-147-implementation-plan.md) — `ContractGeneratorOperations` orchestrator shape; [NEO-148-implementation-plan.md](NEO-148-implementation-plan.md) — `TryDeliverContractCompletion` + outcome audit split +**Precursors:** [NEO-147](https://linear.app/neon-sprawl/issue/NEO-147) **`Done`** — `ContractGeneratorOperations.TryIssue`; [NEO-148](https://linear.app/neon-sprawl/issue/NEO-148) **`Done`** — `RewardRouterOperations.TryDeliverContractCompletion` +**Blocks:** [NEO-151](https://linear.app/neon-sprawl/issue/NEO-151) (E7M4-08 HTTP contract projections + Bruno issue/clear loop) +**Client counterpart:** [NEO-153](https://linear.app/neon-sprawl/issue/NEO-153), [NEO-154](https://linear.app/neon-sprawl/issue/NEO-154) (blocked by E7M4-08; no Godot work in this story) + +## Goal + +Encounter clear completes a matching **active** contract instance and triggers the template **`completionRewardBundle`** payout. First-time encounter loot (E5.M3) remains unchanged and runs **before** contract payout. + +## Kickoff clarifications + +**No blocking decisions needed.** Linear AC, [E7M4-06 backlog scope](E7M4-pre-production-backlog.md#e7m4-06--contractcompletionoperations--encounter-wiring), [E7M4 kickoff decisions table](E7M4-pre-production-backlog.md#kickoff-decisions-decomposition-defaults), and landed NEO-128/NEO-147/NEO-148 code settle: + +| Topic | Decision | Evidence | +|-------|----------|----------| +| Grant order vs encounter loot | **Encounter loot first**, contract payout second — contract wiring runs only after `TryCompleteAndGrant` commits encounter grants + completion mark | Backlog E7M4-06 in-scope wiring note; `EncounterCompletionOperations` already applies reward-table grants before downstream hooks | +| Deliver-then-mark | **`TryDeliverContractCompletion`** then **`IContractInstanceStore.TryMarkComplete`**; on delivery deny, instance stays **`active`** | NEO-128 quest precedent; README reward router § contract wiring note | +| Outcome audit | **`IContractOutcomeStore.TryAppend`** after successful delivery + mark; grant snapshots from **`RewardDeliveryResult`** / **`RewardDeliveryEvent`** | NEO-146 open-questions table; outcome store requires existing instance row | +| Encounter template match | Compare cleared **`encounterId`** to template **`encounterTemplateId`** on the player's active instance | Backlog “match encounterTemplateId”; prototype binds **`prototype_combat_pocket`** | +| No active / wrong encounter | **Structured no-op success** — do not fail encounter completion | Mirror quest wiring best-effort; encounter already committed | +| Idempotent replay | When instance already **`completed`**, return success without re-delivering (router **`already_delivered`** path if invoked) | NEO-128 + store **`TryMarkComplete`** replay semantics | +| Wiring call site | **`EncounterCompletionOperations.TryCompleteAndGrant`** after **`QuestObjectiveWiring.TryProcessEncounterCompleteSuccess`** | Same hook point as quest credit; `EncounterCombatWiring` unchanged except extended dependency pass-through | +| Best-effort | Contract deny does **not** roll back encounter loot or completion mark | Encounter grants committed before contract hook; quest wiring precedent | +| Bruno smoke (`issue → clear → GET`) | **Deferred to NEO-151** — no contract HTTP routes yet | E7M4-08 scope; this story covers orchestrator + encounter wiring + integration tests | +| Contract fixture reset | Add contract delivery **`TryClear`** helper for Bruno/dev loops (NEO-148 deferred item) | NEO-148 plan open questions | +| Telemetry | **Comment-only** `contract_complete` hook in orchestrator — E7M4-09 / NEO-152 | Backlog E7M4-09 | + +## Scope and out-of-scope + +**In scope (from Linear + backlog):** + +- **`ContractCompletionOperations.TryCompleteOnEncounterClear`** static orchestrator in `Game/Contracts/`. +- **`ContractCompletionReasonCodes`** + **`ContractCompletionOperationResult`** envelope. +- Wire from **`EncounterCompletionOperations`** success path (after encounter loot + quest wiring). +- Pass contract dependencies through **`EncounterCombatWiring`** → **`AbilityCastApi`** (mirror quest store injection). +- Flow: resolve active instance → match encounter → deliver bundle → mark instance **`completed`** → append **`ContractOutcomeRow`**. +- Integration tests: issue → encounter clear → instance **`completed`** + delivery row + outcome row; clear without contract unchanged; wrong encounter no-op; encounter first-time loot idempotency unchanged. +- Extend **`QuestFixtureOperations`** (or adjacent dev fixture) with contract delivery **`TryClear`** for test/Bruno reset loops. +- `server/README.md` contract completion operations section; E7.M4 module anchor. + +**Out of scope (from Linear + backlog):** + +- HTTP **`POST …/contracts/issue`**, contract GET projections (NEO-151 / E7M4-08). +- Bruno smoke requiring HTTP (NEO-151). +- **`ContractEconomyValidation`** at issue (NEO-150 / E7M4-07). +- Godot HUD (NEO-153 / NEO-154). +- E9.M1 telemetry ingest (comment-only hook sites optional). + +**Client counterpart:** [NEO-153](https://linear.app/neon-sprawl/issue/NEO-153), [NEO-154](https://linear.app/neon-sprawl/issue/NEO-154) — server HTTP must land in NEO-151 first. + +## Acceptance criteria checklist + +- [ ] Active contract completes when bound encounter clears. +- [ ] Encounter first-time loot idempotency unchanged (E5.M3). + +## Technical approach + +### 1. Types (`Game/Contracts/`) + +| Type | Role | +|------|------| +| **`ContractCompletionReasonCodes`** | Stable codes: **`no_active_contract`**, **`encounter_mismatch`**, **`unknown_template`**, **`player_not_writable`**, **`invalid_ids`**; passthrough router codes on delivery deny (e.g. **`inventory_full`**, **`already_delivered`**) | +| **`ContractCompletionOperationResult`** | `success`, optional `reasonCode`, optional **`ContractInstanceState`** snapshot, optional **`ContractOutcomeRow`** | + +### 2. `ContractCompletionOperations.TryCompleteOnEncounterClear` + +**Inputs:** `playerId`, `encounterId`, injected `IContractInstanceStore`, `IContractOutcomeStore`, `IContractTemplateRegistry`, reward-router dependencies (same set as **`QuestStateOperations.TryMarkComplete`**), `TimeProvider`. + +**Flow:** + +1. Normalize **`playerId`** + **`encounterId`**; deny **`invalid_ids`** when empty. +2. **`instanceStore.TryGetActiveForPlayer`** — no active row ⇒ success no-op (**`no_active_contract`**, or null reason for silent no-op — prefer explicit reason in tests, null in wiring if best-effort silent). +3. Load template via **`templateRegistry.TryGetDefinition(active.TemplateId)`**; deny **`unknown_template`** when missing. +4. Normalize template **`encounterTemplateId`** vs cleared **`encounterId`** — mismatch ⇒ success no-op (**`encounter_mismatch`**). +5. If active snapshot status is already **`completed`** (defensive) ⇒ idempotent success. +6. **`RewardRouterOperations.TryDeliverContractCompletion`** with template **`CompletionRewardBundle`**. +7. On delivery deny ⇒ return deny with router **`reasonCode`**; instance stays **`active`**. +8. **`instanceStore.TryMarkComplete`** with **`timeProvider.GetUtcNow()`**; handle store replay idempotency (completed snapshot ⇒ success). +9. Build **`ContractOutcomeRow`** from delivery grants + **`ContractOutcomeIds.MakeIdempotencyKey`**; **`outcomeStore.TryAppend`** (best-effort if duplicate idempotency key after race — treat as success when instance already completed). +10. Comment-only **`contract_complete`** telemetry hook site (NEO-152). + +**No-op vs deny:** Structural preconditions that mean “this encounter clear is not a contract objective” (**no active**, **encounter mismatch**) return **`success: true`** so encounter completion is never blocked. Delivery/store failures return **`success: false`**. + +### 3. Encounter wiring + +In **`EncounterCompletionOperations.TryCompleteAndGrant`**, after encounter event record + **`QuestObjectiveWiring.TryProcessEncounterCompleteSuccess`**, call: + +```csharp +ContractCompletionOperations.TryCompleteOnEncounterClear( + normalizedPlayerId, + normalizedEncounterId, + /* contract + router deps */); +``` + +Extend **`EncounterCombatWiring.TryProcessCastOutcome`** and **`AbilityCastApi`** to resolve and pass: + +- **`IContractInstanceStore`** +- **`IContractOutcomeStore`** +- **`IContractTemplateRegistry`** + +(Mirror existing quest registry / progress store injection.) + +### 4. Dev fixture reset (NEO-148 follow-on) + +Extend **`QuestFixtureOperations`** reset body (or parallel contract fixture helper) to clear contract delivery rows via **`IRewardDeliveryStore.TryClear(playerId, RewardDeliverySourceKinds.ContractCompletion, contractInstanceId)`** when resetting contract test state — enables future Bruno loops in NEO-151. + +### 5. Documentation + +- **`server/README.md`**: new **Contract completion operations (NEO-149)** section — orchestrator flow, reason codes, deliver-then-mark, outcome append, encounter wiring order. +- Update **`E7_M4_ContractMissionGenerator.md`** Slice 4 anchor when shipped. + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/Contracts/ContractCompletionReasonCodes.cs` | Stable deny / no-op reason codes | +| `server/NeonSprawl.Server/Game/Contracts/ContractCompletionOperationResult.cs` | Orchestrator result envelope | +| `server/NeonSprawl.Server/Game/Contracts/ContractCompletionOperations.cs` | **`TryCompleteOnEncounterClear`** orchestrator | +| `server/NeonSprawl.Server.Tests/Game/Contracts/ContractCompletionOperationsTests.cs` | Unit tests — match/mismatch, deliver-then-mark, idempotent replay, delivery deny leaves active | +| `server/NeonSprawl.Server.Tests/Game/Contracts/ContractCompletionOperationsIntegrationTests.cs` | DI path: issue → encounter complete → completed instance + delivery + outcome | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Game/Encounters/EncounterCompletionOperations.cs` | Call contract completion after quest wiring on successful encounter grant | +| `server/NeonSprawl.Server/Game/Encounters/EncounterCombatWiring.cs` | Pass contract store/registry deps to **`TryCompleteAndGrant`** | +| `server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs` | Resolve contract DI services for combat wiring | +| `server/NeonSprawl.Server.Tests/Game/Encounters/EncounterCompletionOperationsTests.cs` | Update dependency resolution + assert encounter loot unchanged when contract completes | +| `server/NeonSprawl.Server.Tests/Game/Encounters/EncounterCombatWiringTests.cs` | Pass contract deps; optional end-to-end issue → cast → contract completed | +| `server/NeonSprawl.Server/Game/Quests/QuestFixtureOperations.cs` | Contract delivery **`TryClear`** on fixture reset (NEO-148 deferred) | +| `server/README.md` | Contract completion operations + wiring order docs | +| `docs/decomposition/modules/E7_M4_ContractMissionGenerator.md` | NEO-149 completion wiring anchor when shipped | + +## Tests + +| File | Coverage | +|------|----------| +| `ContractCompletionOperationsTests.cs` | **`TryCompleteOnEncounterClear_ShouldCompleteAndDeliver_WhenActiveInstanceMatchesEncounter`** — issue via generator helper, complete on **`prototype_combat_pocket`**, assert **`completed`**, delivery store row, outcome row, bundle grants | +| Same | **`TryCompleteOnEncounterClear_ShouldNoOp_WhenNoActiveContract`** — encounter clear succeeds structurally; no delivery row | +| Same | **`TryCompleteOnEncounterClear_ShouldNoOp_WhenEncounterMismatch`** — active contract bound to combat pocket; clear different encounter id ⇒ still **`active`** | +| Same | **`TryCompleteOnEncounterClear_ShouldLeaveActive_WhenDeliveryDenies`** — full inventory / deny path; instance stays **`active`**, no outcome row | +| Same | **`TryCompleteOnEncounterClear_ShouldBeIdempotent_WhenReplayed`** — second call after complete returns success without double grant | +| `ContractCompletionOperationsIntegrationTests.cs` | Full DI: **`TryIssue`** → **`TryCompleteAndGrant`** → stores consistent via **`InMemoryWebApplicationFactory`** | +| `EncounterCompletionOperationsTests.cs` | **`TryCompleteAndGrant_ShouldGrantPrototypeLootOnce_WhenAllTargetsDefeated`** still passes with contract deps wired; add case with active contract asserting **both** encounter loot (×10 scrap + token) **and** contract bundle (×5 scrap + salvage XP) | +| `EncounterCombatWiringTests.cs` | Update all **`TryProcessCastOutcome`** call sites with contract deps (compile fix) | + +All new tests follow AAA with **`// Arrange` / `// Act` / `// Assert`** labels per [csharp-style](../.cursor/rules/csharp-style.md). + +Manual Godot QA: **none** (server engine; capstone NEO-154). Bruno HTTP loop: **NEO-151**. + +## Open questions / risks + +| Question / risk | Agent recommendation | Status | +|-----------------|----------------------|--------| +| Contract issued after encounter already cleared | Prototype flow is issue-then-clear; edge case leaves contract **`active`** until player can re-trigger (not possible in v1) — document in README; no v1 fix | `adopted` | +| Outcome append fails after mark complete | Treat duplicate idempotency append as success when instance **`completed`**; log at debug if needed | `adopted` | +| Large signature growth on encounter completion | Accept for prototype — mirror quest deps already on **`TryCompleteAndGrant`**; refactor to context object only if a follow-on story demands | `deferred` | +| Bruno smoke in backlog vs no HTTP | Integration tests + fixture reset in this story; Bruno **`issue → clear → GET`** in **NEO-151** | `adopted` | +| **`contract_complete` telemetry | Comment-only hook in orchestrator; NEO-152 fills payload | `deferred` | + +## Client counterpart + +[NEO-153](https://linear.app/neon-sprawl/issue/NEO-153) — contract panel after E7M4-08 HTTP. [NEO-154](https://linear.app/neon-sprawl/issue/NEO-154) — Godot capstone. This story is server-only; Bruno verification of the full player loop lands with NEO-151. From bcf6fd2641702482f91b4275c494a7217fc0812d Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 27 Jun 2026 21:48:17 -0400 Subject: [PATCH 2/7] NEO-149: wire contract completion on encounter clear Add ContractCompletionOperations orchestrator (deliver-then-mark, outcome audit), hook from EncounterCompletionOperations after E5.M3 loot and quest wiring, extend dev fixture reset for contract rows, Bruno quest-fixture reset request, and cover with tests (904 green). --- ...et contract instance via quest fixture.bru | 35 +++ .../scripts/bruno-dev-fixture-helper.js | 15 + .../modules/E7_M4_ContractMissionGenerator.md | 2 + docs/plans/NEO-149-implementation-plan.md | 13 +- ...actCompletionOperationsIntegrationTests.cs | 171 ++++++++++++ .../ContractCompletionOperationsTests.cs | 263 ++++++++++++++++++ .../Encounters/EncounterCombatWiringTests.cs | 41 ++- .../EncounterCompletionOperationsTests.cs | 96 ++++++- .../Game/Quests/QuestObjectiveWiringTests.cs | 10 + .../Game/Quests/QuestProgressApiTests.cs | 10 + .../Game/AbilityInput/AbilityCastApi.cs | 7 + .../ContractCompletionOperationResult.cs | 11 + .../Contracts/ContractCompletionOperations.cs | 197 +++++++++++++ .../ContractCompletionReasonCodes.cs | 15 + .../Game/Encounters/EncounterCombatWiring.cs | 7 + .../EncounterCompletionOperations.cs | 22 ++ .../Game/Quests/QuestFixtureApi.cs | 4 + .../Game/Quests/QuestFixtureDtos.cs | 4 + .../Game/Quests/QuestFixtureOperations.cs | 33 ++- server/README.md | 33 ++- 20 files changed, 981 insertions(+), 8 deletions(-) create mode 100644 bruno/neon-sprawl-server/contract-stores/Reset contract instance via quest fixture.bru create mode 100644 server/NeonSprawl.Server.Tests/Game/Contracts/ContractCompletionOperationsIntegrationTests.cs create mode 100644 server/NeonSprawl.Server.Tests/Game/Contracts/ContractCompletionOperationsTests.cs create mode 100644 server/NeonSprawl.Server/Game/Contracts/ContractCompletionOperationResult.cs create mode 100644 server/NeonSprawl.Server/Game/Contracts/ContractCompletionOperations.cs create mode 100644 server/NeonSprawl.Server/Game/Contracts/ContractCompletionReasonCodes.cs diff --git a/bruno/neon-sprawl-server/contract-stores/Reset contract instance via quest fixture.bru b/bruno/neon-sprawl-server/contract-stores/Reset contract instance via quest fixture.bru new file mode 100644 index 0000000..4ac34c8 --- /dev/null +++ b/bruno/neon-sprawl-server/contract-stores/Reset contract instance via quest fixture.bru @@ -0,0 +1,35 @@ +meta { + name: Reset contract instance via quest fixture (NEO-149) + type: http + seq: 2 +} + +post { + url: {{baseUrl}}/game/players/{{playerId}}/__dev/quest-fixture + body: json + auth: none +} + +body:json { + { + "schemaVersion": 1, + "resetContractInstanceIds": [ + "ci_00000000000000000000000000000000" + ] + } +} + +docs { + NEO-149: quest-fixture accepts resetContractInstanceIds — clears contract instance, contract_completion delivery row, and outcome audit for each id (idempotent when absent). + Full issue → clear → GET Bruno loop lands in NEO-151 when contract HTTP ships. +} + +tests { + test("status 200", function () { + expect(res.getStatus()).to.equal(200); + }); + + test("applied true", function () { + expect(res.getBody().applied).to.equal(true); + }); +} diff --git a/bruno/neon-sprawl-server/scripts/bruno-dev-fixture-helper.js b/bruno/neon-sprawl-server/scripts/bruno-dev-fixture-helper.js index 89ec479..b97bce1 100644 --- a/bruno/neon-sprawl-server/scripts/bruno-dev-fixture-helper.js +++ b/bruno/neon-sprawl-server/scripts/bruno-dev-fixture-helper.js @@ -66,6 +66,20 @@ async function resetPrototypeFactionStanding(bru) { } } +async function resetContractInstances(bru, contractInstanceIds) { + const { baseUrl, playerId, jsonHeaders } = resolveConfig(bru); + const response = await axios.post( + `${baseUrl}/game/players/${playerId}/__dev/quest-fixture`, + { schemaVersion: 1, resetContractInstanceIds: contractInstanceIds }, + jsonHeaders, + ); + if (response.status !== 200 || response.data?.applied !== true) { + throw new Error( + `quest-fixture resetContractInstanceIds failed: ${response.status} ${JSON.stringify(response.data)}`, + ); + } +} + async function assertAllPrototypeQuestsNotStarted(bru) { const { baseUrl, playerId } = resolveConfig(bru); const response = await axios.get(`${baseUrl}/game/players/${playerId}/quest-progress`, { @@ -111,6 +125,7 @@ module.exports = { resetGatherIntroQuestProgress, resetGatherIntroSpine, resetPrototypeFactionStanding, + resetContractInstances, clearInventory, assertAllPrototypeQuestsNotStarted, ALL_PROTOTYPE_QUEST_IDS, diff --git a/docs/decomposition/modules/E7_M4_ContractMissionGenerator.md b/docs/decomposition/modules/E7_M4_ContractMissionGenerator.md index 3bdf964..08e6e0d 100644 --- a/docs/decomposition/modules/E7_M4_ContractMissionGenerator.md +++ b/docs/decomposition/modules/E7_M4_ContractMissionGenerator.md @@ -61,6 +61,8 @@ Epic 7 **Slice 4** — `contract_issued`, `contract_complete`, reward anomalies. **Contract reward router (NEO-148):** **`RewardRouterOperations.TryDeliverContractCompletion`** — idempotent contract **`completionRewardBundle`** apply with key **`{playerId}:contract_complete:{contractInstanceId}`**; [server README — Reward router](../../../server/README.md#reward-router-neo-127). +**Contract completion wiring (NEO-149):** **`ContractCompletionOperations.TryCompleteOnEncounterClear`** — encounter clear completes matching active instance + outcome audit; [server README — Contract completion operations](../../../server/README.md#contract-completion-operations-neo-149). + ## Source anchors - Master plan: [`neon_sprawl_vision.plan.md`](../../../neon_sprawl_vision.plan.md) — Epic 7. diff --git a/docs/plans/NEO-149-implementation-plan.md b/docs/plans/NEO-149-implementation-plan.md index aef0a3a..f5f9a59 100644 --- a/docs/plans/NEO-149-implementation-plan.md +++ b/docs/plans/NEO-149-implementation-plan.md @@ -56,8 +56,17 @@ Encounter clear completes a matching **active** contract instance and triggers t ## Acceptance criteria checklist -- [ ] Active contract completes when bound encounter clears. -- [ ] Encounter first-time loot idempotency unchanged (E5.M3). +- [x] Active contract completes when bound encounter clears. +- [x] Encounter first-time loot idempotency unchanged (E5.M3). + +## Implementation reconciliation (shipped) + +- **Types:** `ContractCompletionReasonCodes`, `ContractCompletionOperationResult`. +- **Orchestrator:** `ContractCompletionOperations.TryCompleteOnEncounterClear` — deliver-then-mark, outcome append, no-op on mismatch/no active. +- **Wiring:** `EncounterCompletionOperations` → contract hook after quest wiring; `EncounterCombatWiring` / `AbilityCastApi` pass contract DI. +- **Fixture:** `QuestFixtureRequest.resetContractInstanceIds` clears instance + delivery + outcome rows. +- **Tests:** 5 unit + 1 integration + 1 encounter wiring test; full suite **904** tests green. +- **Docs:** `server/README.md` contract completion section; E7.M4 module anchor. ## Technical approach diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractCompletionOperationsIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractCompletionOperationsIntegrationTests.cs new file mode 100644 index 0000000..4f729ff --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractCompletionOperationsIntegrationTests.cs @@ -0,0 +1,171 @@ +using Microsoft.Extensions.DependencyInjection; +using NeonSprawl.Server.Game.Contracts; +using NeonSprawl.Server.Game.Encounters; +using NeonSprawl.Server.Game.Factions; +using NeonSprawl.Server.Game.Items; +using NeonSprawl.Server.Game.Mastery; +using NeonSprawl.Server.Game.Quests; +using NeonSprawl.Server.Game.Rewards; +using NeonSprawl.Server.Game.Skills; +using NeonSprawl.Server.Tests; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.Contracts; + +public sealed class ContractCompletionOperationsIntegrationTests +{ + private const string PlayerId = "dev-local-1"; + private const string TemplateId = "prototype_contract_clear_combat_pocket"; + private const string EncounterId = "prototype_combat_pocket"; + private const string MeleeNpc = "prototype_npc_melee"; + private const string RangedNpc = "prototype_npc_ranged"; + private const string EliteNpc = "prototype_npc_elite"; + private const string SeedBucket = "2026-06-22"; + + [Fact] + public async Task TryIssueThenEncounterComplete_ShouldMarkContractCompleted_WhenResolvedFromDi() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + using var client = factory.CreateClient(); + _ = await client.GetAsync("/health"); + var deps = ResolveDependencies(factory); + var issued = ContractGeneratorOperations.TryIssue( + PlayerId, + TemplateId, + SeedBucket, + zoneDifficultyBand: null, + deps.TemplateRegistry, + deps.InstanceStore, + deps.StandingStore, + deps.TimeProvider); + Assert.True(issued.Success); + var instanceId = issued.Snapshot!.ContractInstanceId; + DefeatAllRequiredTargets(deps); + + // Act + var encounterResult = EncounterCompletionOperations.TryCompleteAndGrant( + PlayerId, + EncounterId, + deps.EncounterRegistry, + deps.RewardTableRegistry, + deps.ProgressStore, + deps.CompletionStore, + deps.EventStore, + deps.ItemRegistry, + deps.InventoryStore, + deps.QuestRegistry, + deps.QuestProgressStore, + deps.SkillRegistry, + deps.SkillProgressionStore, + deps.LevelCurve, + deps.PerkUnlockEngine, + deps.PerkStore, + deps.DeliveryStore, + deps.StandingStore, + deps.AuditStore, + deps.InstanceStore, + deps.OutcomeStore, + deps.TemplateRegistry, + deps.TimeProvider); + + // Assert + Assert.True(encounterResult.Success); + Assert.True(deps.InstanceStore.TryGet(PlayerId, instanceId, out var instance)); + Assert.Equal(ContractInstanceStatus.Completed, instance.Status); + Assert.True( + deps.DeliveryStore.TryGet( + PlayerId, + RewardDeliverySourceKinds.ContractCompletion, + instanceId, + out _)); + Assert.Single(deps.OutcomeStore.GetOutcomesForInstance(instanceId)); + deps.InventoryStore.TryGetSnapshot(PlayerId, out var inventory); + Assert.Equal(15, CountBagItem(inventory!, "scrap_metal_bulk")); + Assert.Equal(1, CountBagItem(inventory!, "contract_handoff_token")); + } + + private static void DefeatAllRequiredTargets(IntegrationTestDependencies deps) + { + foreach (var npcId in new[] { MeleeNpc, RangedNpc, EliteNpc }) + { + _ = EncounterProgressOperations.TryActivateOnFirstEngagement( + PlayerId, + npcId, + deps.EncounterRegistry, + deps.ProgressStore, + deps.CompletionStore); + _ = EncounterProgressOperations.TryMarkTargetDefeated( + PlayerId, + npcId, + deps.EncounterRegistry, + deps.ProgressStore, + deps.CompletionStore); + } + } + + private static int CountBagItem(PlayerInventorySnapshot snapshot, string itemId) + { + var total = 0; + foreach (var slot in snapshot.BagSlots) + { + if (string.Equals(slot.ItemId, itemId, StringComparison.Ordinal)) + { + total += slot.Quantity; + } + } + + return total; + } + + private static IntegrationTestDependencies ResolveDependencies(InMemoryWebApplicationFactory factory) + { + using var scope = factory.Services.CreateScope(); + var services = scope.ServiceProvider; + return new IntegrationTestDependencies( + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService()); + } + + private sealed record IntegrationTestDependencies( + IContractTemplateRegistry TemplateRegistry, + IContractInstanceStore InstanceStore, + IContractOutcomeStore OutcomeStore, + IEncounterDefinitionRegistry EncounterRegistry, + IRewardTableDefinitionRegistry RewardTableRegistry, + IEncounterProgressStore ProgressStore, + IEncounterCompletionStore CompletionStore, + IEncounterCompleteEventStore EventStore, + IItemDefinitionRegistry ItemRegistry, + IPlayerInventoryStore InventoryStore, + IQuestDefinitionRegistry QuestRegistry, + IPlayerQuestStateStore QuestProgressStore, + ISkillDefinitionRegistry SkillRegistry, + IPlayerSkillProgressionStore SkillProgressionStore, + ISkillLevelCurve LevelCurve, + PerkUnlockEngine PerkUnlockEngine, + IPlayerPerkStateStore PerkStore, + IRewardDeliveryStore DeliveryStore, + IFactionStandingStore StandingStore, + IReputationDeltaStore AuditStore, + TimeProvider TimeProvider); +} diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractCompletionOperationsTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractCompletionOperationsTests.cs new file mode 100644 index 0000000..4c63022 --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractCompletionOperationsTests.cs @@ -0,0 +1,263 @@ +using Microsoft.Extensions.DependencyInjection; +using NeonSprawl.Server.Game.Contracts; +using NeonSprawl.Server.Game.Factions; +using NeonSprawl.Server.Game.Items; +using NeonSprawl.Server.Game.Mastery; +using NeonSprawl.Server.Game.Rewards; +using NeonSprawl.Server.Game.Skills; +using NeonSprawl.Server.Tests; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.Contracts; + +public sealed class ContractCompletionOperationsTests +{ + private const string PlayerId = "dev-local-1"; + private const string TemplateId = "prototype_contract_clear_combat_pocket"; + private const string EncounterId = "prototype_combat_pocket"; + private const string MismatchEncounterId = "prototype_encounter_other"; + private const string SeedBucket = "2026-06-22"; + private static readonly DateTimeOffset CompletedAt = new(2026, 6, 22, 12, 0, 0, TimeSpan.Zero); + + [Fact] + public async Task TryCompleteOnEncounterClear_ShouldCompleteAndDeliver_WhenActiveInstanceMatchesEncounter() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + var issued = IssuePrototypeContract(deps); + var instanceId = issued.Snapshot!.ContractInstanceId; + deps.InventoryStore.TryGetSnapshot(PlayerId, out var beforeInventory); + + // Act + var result = CompleteOnEncounterClear(deps, EncounterId); + + // Assert + Assert.True(result.Success); + Assert.Null(result.ReasonCode); + Assert.NotNull(result.Snapshot); + Assert.Equal(ContractInstanceStatus.Completed, result.Snapshot!.Status); + Assert.NotNull(result.Outcome); + Assert.True( + deps.DeliveryStore.TryGet( + PlayerId, + RewardDeliverySourceKinds.ContractCompletion, + instanceId, + out var delivery)); + Assert.Equal(RewardDeliverySourceKinds.ContractCompletion, delivery.SourceKind); + Assert.Equal(instanceId, delivery.ContractInstanceId); + Assert.Single(deps.OutcomeStore.GetOutcomesForInstance(instanceId)); + deps.InventoryStore.TryGetSnapshot(PlayerId, out var afterInventory); + Assert.Equal( + CountBagItem(beforeInventory!, "scrap_metal_bulk") + 5, + CountBagItem(afterInventory!, "scrap_metal_bulk")); + var xpAfter = deps.SkillProgressionStore.GetXpTotals(PlayerId); + Assert.True(xpAfter.TryGetValue("salvage", out var salvageXp)); + Assert.Equal(15, salvageXp); + Assert.False(deps.InstanceStore.TryGetActiveForPlayer(PlayerId, out _)); + Assert.True(deps.InstanceStore.TryGet(PlayerId, instanceId, out var stored)); + Assert.Equal(ContractInstanceStatus.Completed, stored.Status); + } + + [Fact] + public async Task TryCompleteOnEncounterClear_ShouldNoOp_WhenNoActiveContract() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + + // Act + var result = CompleteOnEncounterClear(deps, EncounterId); + + // Assert + Assert.True(result.Success); + Assert.Equal(ContractCompletionReasonCodes.NoActiveContract, result.ReasonCode); + Assert.Null(result.Snapshot); + Assert.Null(result.Outcome); + } + + [Fact] + public async Task TryCompleteOnEncounterClear_ShouldNoOp_WhenEncounterMismatch() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + var issued = IssuePrototypeContract(deps); + var instanceId = issued.Snapshot!.ContractInstanceId; + + // Act + var result = CompleteOnEncounterClear(deps, MismatchEncounterId); + + // Assert + Assert.True(result.Success); + Assert.Equal(ContractCompletionReasonCodes.EncounterMismatch, result.ReasonCode); + Assert.NotNull(result.Snapshot); + Assert.Equal(ContractInstanceStatus.Active, result.Snapshot!.Status); + Assert.True(deps.InstanceStore.TryGetActiveForPlayer(PlayerId, out var active)); + Assert.Equal(instanceId, active.ContractInstanceId); + Assert.False( + deps.DeliveryStore.TryGet( + PlayerId, + RewardDeliverySourceKinds.ContractCompletion, + instanceId, + out _)); + } + + [Fact] + public async Task TryCompleteOnEncounterClear_ShouldLeaveActive_WhenDeliveryDenies() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + var issued = IssuePrototypeContract(deps); + var instanceId = issued.Snapshot!.ContractInstanceId; + FillBag(deps); + + // Act + var result = CompleteOnEncounterClear(deps, EncounterId); + + // Assert + Assert.False(result.Success); + Assert.Equal(RewardDeliveryReasonCodes.InventoryFull, result.ReasonCode); + Assert.True(deps.InstanceStore.TryGetActiveForPlayer(PlayerId, out var active)); + Assert.Equal(instanceId, active.ContractInstanceId); + Assert.Equal(ContractInstanceStatus.Active, active.Status); + Assert.Empty(deps.OutcomeStore.GetOutcomesForInstance(instanceId)); + } + + [Fact] + public async Task TryCompleteOnEncounterClear_ShouldBeIdempotent_WhenReplayed() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + var issued = IssuePrototypeContract(deps); + var instanceId = issued.Snapshot!.ContractInstanceId; + var first = CompleteOnEncounterClear(deps, EncounterId); + Assert.True(first.Success); + deps.InventoryStore.TryGetSnapshot(PlayerId, out var afterFirst); + var salvageAfterFirst = deps.SkillProgressionStore.GetXpTotals(PlayerId).GetValueOrDefault("salvage"); + + // Act + var replay = CompleteOnEncounterClear(deps, EncounterId); + + // Assert + Assert.True(replay.Success); + Assert.Equal(ContractCompletionReasonCodes.NoActiveContract, replay.ReasonCode); + deps.InventoryStore.TryGetSnapshot(PlayerId, out var afterReplay); + Assert.Equal( + CountBagItem(afterFirst!, "scrap_metal_bulk"), + CountBagItem(afterReplay!, "scrap_metal_bulk")); + Assert.Equal( + salvageAfterFirst, + deps.SkillProgressionStore.GetXpTotals(PlayerId).GetValueOrDefault("salvage")); + Assert.True( + deps.DeliveryStore.TryGet( + PlayerId, + RewardDeliverySourceKinds.ContractCompletion, + instanceId, + out _)); + } + + private static ContractIssueOperationResult IssuePrototypeContract(CompletionTestDependencies deps) => + ContractGeneratorOperations.TryIssue( + PlayerId, + TemplateId, + SeedBucket, + zoneDifficultyBand: null, + deps.TemplateRegistry, + deps.InstanceStore, + deps.StandingStore, + deps.TimeProvider); + + private static ContractCompletionOperationResult CompleteOnEncounterClear( + CompletionTestDependencies deps, + string encounterId) => + ContractCompletionOperations.TryCompleteOnEncounterClear( + PlayerId, + encounterId, + deps.InstanceStore, + deps.OutcomeStore, + deps.TemplateRegistry, + deps.ItemRegistry, + deps.InventoryStore, + deps.SkillRegistry, + deps.SkillProgressionStore, + deps.LevelCurve, + deps.PerkUnlockEngine, + deps.PerkStore, + deps.DeliveryStore, + deps.StandingStore, + deps.AuditStore, + deps.TimeProvider); + + private static void FillBag(CompletionTestDependencies deps) + { + for (var i = 0; i < PlayerInventorySnapshot.BagSlotCount; i++) + { + var add = PlayerInventoryOperations.TryAddStack( + PlayerId, + "survey_drone_kit", + quantity: 1, + deps.ItemRegistry, + deps.InventoryStore); + Assert.Equal(PlayerInventoryMutationKind.Applied, add.Kind); + } + } + + private static int CountBagItem(PlayerInventorySnapshot snapshot, string itemId) + { + var total = 0; + foreach (var slot in snapshot.BagSlots) + { + if (string.Equals(slot.ItemId, itemId, StringComparison.Ordinal)) + { + total += slot.Quantity; + } + } + + return total; + } + + private static CompletionTestDependencies ResolveDependencies(InMemoryWebApplicationFactory factory) + { + using var scope = factory.Services.CreateScope(); + var services = scope.ServiceProvider; + return new CompletionTestDependencies( + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + new FakeTimeProvider(CompletedAt)); + } + + private sealed class FakeTimeProvider(DateTimeOffset utcNow) : TimeProvider + { + public override DateTimeOffset GetUtcNow() => utcNow; + } + + private sealed record CompletionTestDependencies( + IContractTemplateRegistry TemplateRegistry, + IContractInstanceStore InstanceStore, + IContractOutcomeStore OutcomeStore, + IItemDefinitionRegistry ItemRegistry, + IPlayerInventoryStore InventoryStore, + ISkillDefinitionRegistry SkillRegistry, + IPlayerSkillProgressionStore SkillProgressionStore, + ISkillLevelCurve LevelCurve, + PerkUnlockEngine PerkUnlockEngine, + IPlayerPerkStateStore PerkStore, + IRewardDeliveryStore DeliveryStore, + IFactionStandingStore StandingStore, + IReputationDeltaStore AuditStore, + TimeProvider TimeProvider); +} diff --git a/server/NeonSprawl.Server.Tests/Game/Encounters/EncounterCombatWiringTests.cs b/server/NeonSprawl.Server.Tests/Game/Encounters/EncounterCombatWiringTests.cs index 3361d16..1508193 100644 --- a/server/NeonSprawl.Server.Tests/Game/Encounters/EncounterCombatWiringTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Encounters/EncounterCombatWiringTests.cs @@ -1,4 +1,5 @@ using Microsoft.Extensions.DependencyInjection; +using NeonSprawl.Server.Game.Contracts; using NeonSprawl.Server.Game.Factions; using NeonSprawl.Server.Game.Encounters; using NeonSprawl.Server.Game.Items; @@ -49,6 +50,9 @@ public sealed class EncounterCombatWiringTests deps.DeliveryStore, deps.StandingStore, deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, TimeProvider.System); // Assert @@ -88,6 +92,9 @@ public sealed class EncounterCombatWiringTests deps.DeliveryStore, deps.StandingStore, deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, TimeProvider.System); // Assert @@ -131,6 +138,9 @@ public sealed class EncounterCombatWiringTests deps.DeliveryStore, deps.StandingStore, deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, TimeProvider.System); // Assert @@ -172,6 +182,9 @@ public sealed class EncounterCombatWiringTests deps.DeliveryStore, deps.StandingStore, deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, TimeProvider.System); // Assert @@ -206,6 +219,9 @@ public sealed class EncounterCombatWiringTests deps.DeliveryStore, deps.StandingStore, deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, TimeProvider.System); deps.InventoryStore.TryGetSnapshot(PlayerId, out var before); @@ -232,6 +248,9 @@ public sealed class EncounterCombatWiringTests deps.DeliveryStore, deps.StandingStore, deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, TimeProvider.System); // Assert @@ -272,6 +291,9 @@ public sealed class EncounterCombatWiringTests deps.DeliveryStore, deps.StandingStore, deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, TimeProvider.System); // Assert @@ -321,6 +343,9 @@ public sealed class EncounterCombatWiringTests deps.DeliveryStore, deps.StandingStore, deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, TimeProvider.System); // Assert @@ -367,6 +392,9 @@ public sealed class EncounterCombatWiringTests deps.DeliveryStore, deps.StandingStore, deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, TimeProvider.System); // Assert @@ -397,6 +425,9 @@ public sealed class EncounterCombatWiringTests deps.DeliveryStore, deps.StandingStore, deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, TimeProvider.System); } @@ -455,7 +486,10 @@ public sealed class EncounterCombatWiringTests services.GetRequiredService(), services.GetRequiredService(), services.GetRequiredService(), - services.GetRequiredService()); + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService()); } private static int CountBagItem(PlayerInventorySnapshot snapshot, string itemId) @@ -489,5 +523,8 @@ public sealed class EncounterCombatWiringTests IPlayerPerkStateStore PerkStore, IRewardDeliveryStore DeliveryStore, IFactionStandingStore StandingStore, - IReputationDeltaStore AuditStore); + IReputationDeltaStore AuditStore, + IContractInstanceStore ContractInstanceStore, + IContractOutcomeStore ContractOutcomeStore, + IContractTemplateRegistry ContractTemplateRegistry); } diff --git a/server/NeonSprawl.Server.Tests/Game/Encounters/EncounterCompletionOperationsTests.cs b/server/NeonSprawl.Server.Tests/Game/Encounters/EncounterCompletionOperationsTests.cs index 3f2d1fd..9682d2e 100644 --- a/server/NeonSprawl.Server.Tests/Game/Encounters/EncounterCompletionOperationsTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Encounters/EncounterCompletionOperationsTests.cs @@ -1,4 +1,5 @@ using Microsoft.Extensions.DependencyInjection; +using NeonSprawl.Server.Game.Contracts; using NeonSprawl.Server.Game.Factions; using NeonSprawl.Server.Game.Encounters; using NeonSprawl.Server.Game.Items; @@ -19,6 +20,67 @@ public sealed class EncounterCompletionOperationsTests private const string RangedNpc = "prototype_npc_ranged"; private const string EliteNpc = "prototype_npc_elite"; + [Fact] + public async Task TryCompleteAndGrant_ShouldApplyContractBundle_WhenActiveContractMatchesEncounter() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + var issued = ContractGeneratorOperations.TryIssue( + PlayerId, + "prototype_contract_clear_combat_pocket", + "2026-06-22", + zoneDifficultyBand: null, + deps.ContractTemplateRegistry, + deps.ContractInstanceStore, + deps.StandingStore, + TimeProvider.System); + Assert.True(issued.Success); + var instanceId = issued.Snapshot!.ContractInstanceId; + DefeatAllRequiredTargets(deps); + + // Act + var result = EncounterCompletionOperations.TryCompleteAndGrant( + PlayerId, + EncounterId, + deps.EncounterRegistry, + deps.RewardTableRegistry, + deps.ProgressStore, + deps.CompletionStore, + deps.EventStore, + deps.ItemRegistry, + deps.InventoryStore, + deps.QuestRegistry, + deps.QuestProgressStore, + deps.SkillRegistry, + deps.SkillProgressionStore, + deps.LevelCurve, + deps.PerkUnlockEngine, + deps.PerkStore, + deps.DeliveryStore, + deps.StandingStore, + deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, + TimeProvider.System); + + // Assert + Assert.True(result.Success); + deps.InventoryStore.TryGetSnapshot(PlayerId, out var inventory); + Assert.Equal(15, CountBagItem(inventory!, "scrap_metal_bulk")); + Assert.Equal(1, CountBagItem(inventory!, "contract_handoff_token")); + Assert.True(deps.ContractInstanceStore.TryGet(PlayerId, instanceId, out var instance)); + Assert.Equal(ContractInstanceStatus.Completed, instance.Status); + Assert.True( + deps.DeliveryStore.TryGet( + PlayerId, + RewardDeliverySourceKinds.ContractCompletion, + instanceId, + out _)); + Assert.Single(deps.ContractOutcomeStore.GetOutcomesForInstance(instanceId)); + } + [Fact] public async Task TryCompleteAndGrant_ShouldGrantPrototypeLootOnce_WhenAllTargetsDefeated() { @@ -49,6 +111,9 @@ public sealed class EncounterCompletionOperationsTests deps.DeliveryStore, deps.StandingStore, deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, timeProvider); // Assert @@ -100,6 +165,9 @@ public sealed class EncounterCompletionOperationsTests deps.DeliveryStore, deps.StandingStore, deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, timeProvider); deps.InventoryStore.TryGetSnapshot(PlayerId, out var afterFirst); @@ -124,6 +192,9 @@ public sealed class EncounterCompletionOperationsTests deps.DeliveryStore, deps.StandingStore, deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, timeProvider); // Assert @@ -190,6 +261,9 @@ public sealed class EncounterCompletionOperationsTests deps.DeliveryStore, deps.StandingStore, deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, TimeProvider.System); // Assert @@ -232,6 +306,9 @@ public sealed class EncounterCompletionOperationsTests deps.DeliveryStore, deps.StandingStore, deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, TimeProvider.System); // Assert @@ -276,6 +353,9 @@ public sealed class EncounterCompletionOperationsTests deps.DeliveryStore, deps.StandingStore, deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, TimeProvider.System); // Assert @@ -318,6 +398,9 @@ public sealed class EncounterCompletionOperationsTests deps.DeliveryStore, deps.StandingStore, deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, TimeProvider.System); // Assert @@ -358,6 +441,9 @@ public sealed class EncounterCompletionOperationsTests deps.DeliveryStore, deps.StandingStore, deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, TimeProvider.System); // Assert @@ -428,7 +514,10 @@ public sealed class EncounterCompletionOperationsTests services.GetRequiredService(), services.GetRequiredService(), services.GetRequiredService(), - services.GetRequiredService()); + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService()); } private static int CountBagItem(PlayerInventorySnapshot snapshot, string itemId) @@ -476,7 +565,10 @@ public sealed class EncounterCompletionOperationsTests IPlayerPerkStateStore PerkStore, IRewardDeliveryStore DeliveryStore, IFactionStandingStore StandingStore, - IReputationDeltaStore AuditStore); + IReputationDeltaStore AuditStore, + IContractInstanceStore ContractInstanceStore, + IContractOutcomeStore ContractOutcomeStore, + IContractTemplateRegistry ContractTemplateRegistry); private sealed class CompletionStoreDeniesMark(IEncounterCompletionStore inner) : IEncounterCompletionStore { diff --git a/server/NeonSprawl.Server.Tests/Game/Quests/QuestObjectiveWiringTests.cs b/server/NeonSprawl.Server.Tests/Game/Quests/QuestObjectiveWiringTests.cs index 1d7ae70..1debd92 100644 --- a/server/NeonSprawl.Server.Tests/Game/Quests/QuestObjectiveWiringTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Quests/QuestObjectiveWiringTests.cs @@ -1,4 +1,5 @@ using Microsoft.Extensions.DependencyInjection; +using NeonSprawl.Server.Game.Contracts; using NeonSprawl.Server.Game.Crafting; using NeonSprawl.Server.Game.Encounters; using NeonSprawl.Server.Game.Factions; @@ -270,6 +271,9 @@ public sealed class QuestObjectiveWiringTests deps.DeliveryStore, deps.StandingStore, deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, deps.TimeProvider); Assert.True(result.Success); } @@ -459,6 +463,9 @@ public sealed class QuestObjectiveWiringTests services.GetRequiredService(), services.GetRequiredService(), services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), TimeProvider.System); } @@ -483,5 +490,8 @@ public sealed class QuestObjectiveWiringTests IRewardDeliveryStore DeliveryStore, IFactionStandingStore StandingStore, IReputationDeltaStore AuditStore, + IContractInstanceStore ContractInstanceStore, + IContractOutcomeStore ContractOutcomeStore, + IContractTemplateRegistry ContractTemplateRegistry, TimeProvider TimeProvider); } diff --git a/server/NeonSprawl.Server.Tests/Game/Quests/QuestProgressApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Quests/QuestProgressApiTests.cs index 258bb65..b081f91 100644 --- a/server/NeonSprawl.Server.Tests/Game/Quests/QuestProgressApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Quests/QuestProgressApiTests.cs @@ -1,6 +1,7 @@ using System.Net; using System.Net.Http.Json; using Microsoft.Extensions.DependencyInjection; +using NeonSprawl.Server.Game.Contracts; using NeonSprawl.Server.Game.Crafting; using NeonSprawl.Server.Game.Encounters; using NeonSprawl.Server.Game.Factions; @@ -412,6 +413,9 @@ public sealed class QuestProgressApiTests deps.DeliveryStore, deps.StandingStore, deps.AuditStore, + deps.ContractInstanceStore, + deps.ContractOutcomeStore, + deps.ContractTemplateRegistry, deps.TimeProvider); Assert.True(result.Success); } @@ -552,6 +556,9 @@ public sealed class QuestProgressApiTests services.GetRequiredService(), services.GetRequiredService(), services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), + services.GetRequiredService(), TimeProvider.System); } @@ -576,5 +583,8 @@ public sealed class QuestProgressApiTests IRewardDeliveryStore DeliveryStore, IFactionStandingStore StandingStore, IReputationDeltaStore AuditStore, + IContractInstanceStore ContractInstanceStore, + IContractOutcomeStore ContractOutcomeStore, + IContractTemplateRegistry ContractTemplateRegistry, TimeProvider TimeProvider); } diff --git a/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs b/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs index 6b4cd95..49099d7 100644 --- a/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs +++ b/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs @@ -1,5 +1,6 @@ using NeonSprawl.Server.Diagnostics; using NeonSprawl.Server.Game.Combat; +using NeonSprawl.Server.Game.Contracts; using NeonSprawl.Server.Game.Encounters; using NeonSprawl.Server.Game.Factions; using NeonSprawl.Server.Game.Gigs; @@ -93,6 +94,9 @@ public static class AbilityCastApi IRewardDeliveryStore deliveryStore, IFactionStandingStore standingStore, IReputationDeltaStore auditStore, + IContractInstanceStore contractInstanceStore, + IContractOutcomeStore contractOutcomeStore, + IContractTemplateRegistry contractTemplateRegistry, ILoggerFactory loggerFactory, TimeProvider clock) => { @@ -302,6 +306,9 @@ public static class AbilityCastApi deliveryStore, standingStore, auditStore, + contractInstanceStore, + contractOutcomeStore, + contractTemplateRegistry, clock, loggerFactory.CreateLogger(nameof(EncounterCombatWiring))); diff --git a/server/NeonSprawl.Server/Game/Contracts/ContractCompletionOperationResult.cs b/server/NeonSprawl.Server/Game/Contracts/ContractCompletionOperationResult.cs new file mode 100644 index 0000000..2fc7beb --- /dev/null +++ b/server/NeonSprawl.Server/Game/Contracts/ContractCompletionOperationResult.cs @@ -0,0 +1,11 @@ +namespace NeonSprawl.Server.Game.Contracts; + +/// +/// Server-internal contract completion resolution envelope (NEO-149). +/// HTTP DTOs via E7M4-08 (NEO-151). +/// +public readonly record struct ContractCompletionOperationResult( + bool Success, + string? ReasonCode, + ContractInstanceState? Snapshot, + ContractOutcomeRow? Outcome); diff --git a/server/NeonSprawl.Server/Game/Contracts/ContractCompletionOperations.cs b/server/NeonSprawl.Server/Game/Contracts/ContractCompletionOperations.cs new file mode 100644 index 0000000..c18909a --- /dev/null +++ b/server/NeonSprawl.Server/Game/Contracts/ContractCompletionOperations.cs @@ -0,0 +1,197 @@ +using NeonSprawl.Server.Game.Factions; +using NeonSprawl.Server.Game.Items; +using NeonSprawl.Server.Game.Mastery; +using NeonSprawl.Server.Game.Rewards; +using NeonSprawl.Server.Game.Skills; + +namespace NeonSprawl.Server.Game.Contracts; + +/// +/// Server-authoritative contract completion on encounter clear (NEO-149). +/// HTTP: E7M4-08 (NEO-151). Telemetry hook sites: E7M4-09 (NEO-152). +/// +public static class ContractCompletionOperations +{ + /// + /// When the player has an active contract bound to , delivers the template + /// completion bundle then marks the instance complete and appends an outcome audit row. + /// + public static ContractCompletionOperationResult TryCompleteOnEncounterClear( + string playerId, + string encounterId, + IContractInstanceStore instanceStore, + IContractOutcomeStore outcomeStore, + IContractTemplateRegistry templateRegistry, + IItemDefinitionRegistry itemRegistry, + IPlayerInventoryStore inventoryStore, + ISkillDefinitionRegistry skillRegistry, + IPlayerSkillProgressionStore skillProgressionStore, + ISkillLevelCurve levelCurve, + PerkUnlockEngine perkUnlockEngine, + IPlayerPerkStateStore perkStore, + IRewardDeliveryStore deliveryStore, + IFactionStandingStore standingStore, + IReputationDeltaStore auditStore, + TimeProvider timeProvider) + { + var normalizedPlayerId = ContractInstanceIds.NormalizePlayerId(playerId); + var normalizedEncounterId = NormalizeEncounterId(encounterId); + if (normalizedPlayerId.Length == 0 || normalizedEncounterId.Length == 0) + { + return Deny(ContractCompletionReasonCodes.InvalidIds); + } + + if (!instanceStore.TryGetActiveForPlayer(normalizedPlayerId, out var activeSnapshot)) + { + return NoOp(ContractCompletionReasonCodes.NoActiveContract); + } + + if (!templateRegistry.TryGetDefinition(activeSnapshot.TemplateId, out var template)) + { + return Deny(ContractCompletionReasonCodes.UnknownTemplate, activeSnapshot); + } + + if (!EncounterMatchesTemplate(normalizedEncounterId, template.EncounterTemplateId)) + { + return NoOp(ContractCompletionReasonCodes.EncounterMismatch, activeSnapshot); + } + + if (activeSnapshot.Status == ContractInstanceStatus.Completed) + { + return Success(activeSnapshot, TryGetExistingOutcome(outcomeStore, activeSnapshot)); + } + + if (!instanceStore.CanWritePlayer(normalizedPlayerId)) + { + return Deny(ContractCompletionReasonCodes.PlayerNotWritable, activeSnapshot); + } + + var delivery = RewardRouterOperations.TryDeliverContractCompletion( + normalizedPlayerId, + activeSnapshot.ContractInstanceId, + template.CompletionRewardBundle, + itemRegistry, + inventoryStore, + skillRegistry, + skillProgressionStore, + levelCurve, + perkUnlockEngine, + perkStore, + deliveryStore, + standingStore, + auditStore, + timeProvider); + + if (!delivery.Success) + { + return Deny(delivery.ReasonCode ?? RewardDeliveryReasonCodes.InvalidContractInstanceId, activeSnapshot); + } + + var completedAt = timeProvider.GetUtcNow(); + if (instanceStore.TryMarkComplete( + normalizedPlayerId, + activeSnapshot.ContractInstanceId, + completedAt, + out var completedSnapshot)) + { + var outcome = AppendOutcome( + outcomeStore, + completedSnapshot, + delivery, + completedAt); + // --- Telemetry hook site (NEO-152): future E9.M1 catalog event `contract_complete` --- + // TODO(E9.M1): catalog emit — first-time completion only; idempotent replay returns below without hook. + // Planned payload: playerId, contractInstanceId, templateId, encounterId, completedAt, idempotencyKey. + return Success(completedSnapshot, outcome); + } + + if (completedSnapshot?.Status == ContractInstanceStatus.Completed) + { + return Success(completedSnapshot, TryGetExistingOutcome(outcomeStore, completedSnapshot)); + } + + if (instanceStore.TryGet( + normalizedPlayerId, + activeSnapshot.ContractInstanceId, + out var afterAttempt) && + afterAttempt.Status == ContractInstanceStatus.Completed) + { + return Success(afterAttempt, TryGetExistingOutcome(outcomeStore, afterAttempt)); + } + + return Deny(ContractCompletionReasonCodes.InvalidIds, activeSnapshot); + } + + private static ContractOutcomeRow? AppendOutcome( + IContractOutcomeStore outcomeStore, + ContractInstanceState snapshot, + RewardDeliveryResult delivery, + DateTimeOffset recordedAt) + { + var idempotencyKey = ContractOutcomeIds.MakeIdempotencyKey( + snapshot.PlayerId, + snapshot.ContractInstanceId); + if (idempotencyKey.Length == 0) + { + return null; + } + + var row = new ContractOutcomeRow( + MakeOutcomeRowId(idempotencyKey), + snapshot.ContractInstanceId, + snapshot.PlayerId, + idempotencyKey, + delivery.GrantedItems, + delivery.GrantedSkillXp, + delivery.GrantedReputation, + recordedAt); + + if (outcomeStore.TryAppend(row)) + { + return row; + } + + return outcomeStore.TryGetByIdempotencyKey(idempotencyKey, out var existing) + ? existing + : null; + } + + private static ContractOutcomeRow? TryGetExistingOutcome( + IContractOutcomeStore outcomeStore, + ContractInstanceState snapshot) + { + var idempotencyKey = ContractOutcomeIds.MakeIdempotencyKey( + snapshot.PlayerId, + snapshot.ContractInstanceId); + return idempotencyKey.Length > 0 && + outcomeStore.TryGetByIdempotencyKey(idempotencyKey, out var existing) + ? existing + : null; + } + + private static string MakeOutcomeRowId(string idempotencyKey) => $"{idempotencyKey}:outcome"; + + private static bool EncounterMatchesTemplate(string normalizedEncounterId, string templateEncounterId) => + string.Equals( + normalizedEncounterId, + NormalizeEncounterId(templateEncounterId), + StringComparison.Ordinal); + + private static string NormalizeEncounterId(string? encounterId) => + encounterId?.Trim() ?? string.Empty; + + private static ContractCompletionOperationResult NoOp( + string reasonCode, + ContractInstanceState? snapshot = null) => + new(true, reasonCode, snapshot, null); + + private static ContractCompletionOperationResult Success( + ContractInstanceState snapshot, + ContractOutcomeRow? outcome) => + new(true, null, snapshot, outcome); + + private static ContractCompletionOperationResult Deny( + string reasonCode, + ContractInstanceState? snapshot = null) => + new(false, reasonCode, snapshot, null); +} diff --git a/server/NeonSprawl.Server/Game/Contracts/ContractCompletionReasonCodes.cs b/server/NeonSprawl.Server/Game/Contracts/ContractCompletionReasonCodes.cs new file mode 100644 index 0000000..a6bc319 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Contracts/ContractCompletionReasonCodes.cs @@ -0,0 +1,15 @@ +namespace NeonSprawl.Server.Game.Contracts; + +/// Stable reason codes for (NEO-149). +public static class ContractCompletionReasonCodes +{ + public const string InvalidIds = ContractInstanceReasonCodes.InvalidIds; + + public const string PlayerNotWritable = ContractInstanceReasonCodes.PlayerNotWritable; + + public const string NoActiveContract = "no_active_contract"; + + public const string EncounterMismatch = "encounter_mismatch"; + + public const string UnknownTemplate = "unknown_template"; +} diff --git a/server/NeonSprawl.Server/Game/Encounters/EncounterCombatWiring.cs b/server/NeonSprawl.Server/Game/Encounters/EncounterCombatWiring.cs index 2799067..2f08862 100644 --- a/server/NeonSprawl.Server/Game/Encounters/EncounterCombatWiring.cs +++ b/server/NeonSprawl.Server/Game/Encounters/EncounterCombatWiring.cs @@ -1,3 +1,4 @@ +using NeonSprawl.Server.Game.Contracts; using NeonSprawl.Server.Game.Factions; using NeonSprawl.Server.Game.Items; using NeonSprawl.Server.Game.Mastery; @@ -38,6 +39,9 @@ public static class EncounterCombatWiring IRewardDeliveryStore deliveryStore, IFactionStandingStore standingStore, IReputationDeltaStore auditStore, + IContractInstanceStore contractInstanceStore, + IContractOutcomeStore contractOutcomeStore, + IContractTemplateRegistry contractTemplateRegistry, TimeProvider timeProvider, ILogger? logger = null) { @@ -104,6 +108,9 @@ public static class EncounterCombatWiring deliveryStore, standingStore, auditStore, + contractInstanceStore, + contractOutcomeStore, + contractTemplateRegistry, timeProvider); if (!completion.Success) diff --git a/server/NeonSprawl.Server/Game/Encounters/EncounterCompletionOperations.cs b/server/NeonSprawl.Server/Game/Encounters/EncounterCompletionOperations.cs index b45350e..17cfa2e 100644 --- a/server/NeonSprawl.Server/Game/Encounters/EncounterCompletionOperations.cs +++ b/server/NeonSprawl.Server/Game/Encounters/EncounterCompletionOperations.cs @@ -1,3 +1,4 @@ +using NeonSprawl.Server.Game.Contracts; using NeonSprawl.Server.Game.Factions; using NeonSprawl.Server.Game.Items; using NeonSprawl.Server.Game.Mastery; @@ -41,6 +42,9 @@ public static class EncounterCompletionOperations IRewardDeliveryStore deliveryStore, IFactionStandingStore standingStore, IReputationDeltaStore auditStore, + IContractInstanceStore contractInstanceStore, + IContractOutcomeStore contractOutcomeStore, + IContractTemplateRegistry contractTemplateRegistry, TimeProvider timeProvider) { if (!encounterRegistry.TryNormalizeKnown(encounterId, out var normalizedEncounterId) || @@ -167,6 +171,24 @@ public static class EncounterCompletionOperations auditStore, timeProvider); + _ = ContractCompletionOperations.TryCompleteOnEncounterClear( + normalizedPlayerId, + normalizedEncounterId, + contractInstanceStore, + contractOutcomeStore, + contractTemplateRegistry, + itemRegistry, + inventoryStore, + skillRegistry, + skillProgressionStore, + levelCurve, + perkUnlockEngine, + perkStore, + deliveryStore, + standingStore, + auditStore, + timeProvider); + return new EncounterCompletionResult( Success: true, ReasonCode: null, diff --git a/server/NeonSprawl.Server/Game/Quests/QuestFixtureApi.cs b/server/NeonSprawl.Server/Game/Quests/QuestFixtureApi.cs index d5e9ce7..6c11780 100644 --- a/server/NeonSprawl.Server/Game/Quests/QuestFixtureApi.cs +++ b/server/NeonSprawl.Server/Game/Quests/QuestFixtureApi.cs @@ -1,3 +1,4 @@ +using NeonSprawl.Server.Game.Contracts; using NeonSprawl.Server.Game.Factions; using NeonSprawl.Server.Game.PositionState; using NeonSprawl.Server.Game.Rewards; @@ -15,6 +16,7 @@ public static class QuestFixtureApi IQuestDefinitionRegistry questRegistry, IFactionDefinitionRegistry factionRegistry, IPlayerQuestStateStore progressStore, IFactionStandingStore standingStore, IRewardDeliveryStore deliveryStore, IReputationDeltaStore auditStore, + IContractInstanceStore contractInstanceStore, IContractOutcomeStore contractOutcomeStore, TimeProvider timeProvider) => { if (body is null || body.SchemaVersion != QuestFixtureRequest.CurrentSchemaVersion) @@ -37,6 +39,8 @@ public static class QuestFixtureApi standingStore, deliveryStore, auditStore, + contractInstanceStore, + contractOutcomeStore, timeProvider)) { return Results.NotFound(); diff --git a/server/NeonSprawl.Server/Game/Quests/QuestFixtureDtos.cs b/server/NeonSprawl.Server/Game/Quests/QuestFixtureDtos.cs index 3f8141e..cc0b207 100644 --- a/server/NeonSprawl.Server/Game/Quests/QuestFixtureDtos.cs +++ b/server/NeonSprawl.Server/Game/Quests/QuestFixtureDtos.cs @@ -21,6 +21,10 @@ public sealed class QuestFixtureRequest /// Faction ids to clear from player standing (dev Bruno reset; missing row reads as 0). [JsonPropertyName("resetFactionIds")] public IReadOnlyList ResetFactionIds { get; init; } = []; + + /// Contract instance ids to clear from instance, delivery, and outcome stores (dev Bruno reset). + [JsonPropertyName("resetContractInstanceIds")] + public IReadOnlyList ResetContractInstanceIds { get; init; } = []; } /// POST response for quest fixture apply. diff --git a/server/NeonSprawl.Server/Game/Quests/QuestFixtureOperations.cs b/server/NeonSprawl.Server/Game/Quests/QuestFixtureOperations.cs index 90b7b1b..8cec549 100644 --- a/server/NeonSprawl.Server/Game/Quests/QuestFixtureOperations.cs +++ b/server/NeonSprawl.Server/Game/Quests/QuestFixtureOperations.cs @@ -1,5 +1,6 @@ namespace NeonSprawl.Server.Game.Quests; +using NeonSprawl.Server.Game.Contracts; using NeonSprawl.Server.Game.Factions; using NeonSprawl.Server.Game.Rewards; @@ -19,11 +20,14 @@ public static class QuestFixtureOperations IFactionStandingStore standingStore, IRewardDeliveryStore deliveryStore, IReputationDeltaStore auditStore, + IContractInstanceStore instanceStore, + IContractOutcomeStore outcomeStore, TimeProvider timeProvider) { if (body.CompletedQuestIds.Count == 0 && body.ResetQuestIds.Count == 0 && - body.ResetFactionIds.Count == 0) + body.ResetFactionIds.Count == 0 && + body.ResetContractInstanceIds.Count == 0) { return true; } @@ -69,6 +73,33 @@ public static class QuestFixtureOperations } } + foreach (var contractInstanceId in body.ResetContractInstanceIds) + { + var normalizedInstanceId = ContractInstanceIds.NormalizeContractInstanceId(contractInstanceId); + if (normalizedInstanceId.Length == 0) + { + return false; + } + + if (!deliveryStore.TryClear( + playerId, + RewardDeliverySourceKinds.ContractCompletion, + normalizedInstanceId)) + { + return false; + } + + if (!instanceStore.TryClearInstance(playerId, normalizedInstanceId)) + { + return false; + } + + if (!outcomeStore.TryClearForInstance(normalizedInstanceId)) + { + return false; + } + } + foreach (var questId in body.CompletedQuestIds) { if (!questRegistry.TryNormalizeKnown(questId, out var normalizedQuestId)) diff --git a/server/README.md b/server/README.md index aa9b8d0..2d45829 100644 --- a/server/README.md +++ b/server/README.md @@ -314,6 +314,37 @@ Completed instances are immutable (no status regression). Player, template, and Economy band-cap lint at issue time lands in **NEO-150** (E7M4-07). HTTP **`POST …/contracts/issue`** lands in **NEO-151** (E7M4-08). Plan: [NEO-147 implementation plan](../../docs/plans/NEO-147-implementation-plan.md). +## Contract completion operations (NEO-149) + +**`ContractCompletionOperations.TryCompleteOnEncounterClear`** completes an **`active`** contract when the cleared encounter matches the template **`encounterTemplateId`**. Call this orchestrator (or rely on encounter wiring) — not stores or the reward router directly. + +**Wiring order:** **`EncounterCompletionOperations.TryCompleteAndGrant`** applies first-time encounter loot (E5.M3), records **`EncounterCompleteEvent`**, runs **`QuestObjectiveWiring`**, then invokes contract completion. Encounter success is never blocked by contract payout failure. + +**Flow:** + +1. **`TryGetActiveForPlayer`** — no active row ⇒ success no-op (**`no_active_contract`**). +2. Load template; deny **`unknown_template`** when missing. +3. Encounter id mismatch ⇒ success no-op (**`encounter_mismatch`**). +4. **`RewardRouterOperations.TryDeliverContractCompletion`** (deliver-then-mark). +5. On delivery deny, instance stays **`active`**; router **`reasonCode`** passthrough (e.g. **`inventory_full`**). +6. **`IContractInstanceStore.TryMarkComplete`** then **`IContractOutcomeStore.TryAppend`** with grant snapshots from the delivery result. + +**Reason codes:** + +| Code | When | +|------|------| +| **`no_active_contract`** | Success no-op — player has no active instance | +| **`encounter_mismatch`** | Success no-op — cleared encounter is not the template objective | +| **`unknown_template`** | Active instance references a missing template id | +| **`invalid_ids`**, **`player_not_writable`** | Normalization / write gate failures | +| *(router passthrough)* | Delivery deny — e.g. **`inventory_full`**, **`already_delivered`** | + +**Dev fixture reset:** **`POST …/__dev/quest-fixture`** accepts optional **`resetContractInstanceIds`** — clears contract instance, **`contract_completion`** delivery row, and outcome audit for each id (NEO-149). + +**Prototype edge case:** issuing a contract after the bound encounter is already cleared leaves the instance **`active`** with no re-trigger in v1 — prototype flow is issue-then-clear. + +HTTP contract GET/issue projections land in **NEO-151** (E7M4-08). Plan: [NEO-149 implementation plan](../../docs/plans/NEO-149-implementation-plan.md). + ## Contract outcome store (NEO-146) Append-only contract completion audit rows live in **`IContractOutcomeStore`**. Each **`ContractOutcomeRow`** captures instance id, delivery idempotency key **`{playerId}:contract_complete:{contractInstanceId}`**, grant snapshot summary (items, skill XP, reputation — same shapes as **`RewardDeliveryEvent`**), and **`recordedAt`**. @@ -491,7 +522,7 @@ Encounter loot remains on **`IEncounterCompleteEventStore`** (E5.M3); quest and | **`progression_store_missing`** | Skill progression store could not apply XP delta. | | **`unknown_faction`**, **`invalid_delta`**, **`audit_append_failed`**, **`invalid_delta_id`**, **`invalid_source`** | Reputation grant denied via **`ReputationOperations`** (NEO-138). | -Apply order: **items first**, then **skill XP**, then **reputation**. Any deny fails closed without store record (compensating rollback of all prior grant types on partial apply). Reputation **`deltaId`**: **`{idempotencyKey}:rep:{factionId}`**; race/compensating revert uses **`:rollback`** suffix on inverse delta. Idempotent replay checks **`TryGet`** before apply. If grants succeed but **`TryRecord`** returns `false` (concurrent race), this call **rolls back** item grants, reverts skill XP via **`SkillProgressionGrantOperations.TryRevertAppliedSkillGrants`**, and reverts reputation via inverse **`TryApplyDelta`** — then returns the existing store event (mirror **`EncounterCompletionOperations`** compensating rollback when **`TryMarkCompleted`** loses). **Quest-state wiring (NEO-128):** **`QuestStateOperations.TryMarkComplete`** and **`QuestObjectiveWiring`** terminal-step completion call **`TryDeliverQuestCompletion`** before **`IPlayerQuestStateStore.TryMarkComplete`**. **Contract wiring (NEO-149):** **`ContractCompletionOperations`** will call **`TryDeliverContractCompletion`**. Bruno quest-flow smoke: `bruno/neon-sprawl-server/quest-progress/Accept grid contract after operator chain.bru` (NEO-138). Plans: [NEO-127](../../docs/plans/NEO-127-implementation-plan.md), [NEO-128](../../docs/plans/NEO-128-implementation-plan.md), [NEO-138](../../docs/plans/NEO-138-implementation-plan.md), [NEO-148](../../docs/plans/NEO-148-implementation-plan.md). +Apply order: **items first**, then **skill XP**, then **reputation**. Any deny fails closed without store record (compensating rollback of all prior grant types on partial apply). Reputation **`deltaId`**: **`{idempotencyKey}:rep:{factionId}`**; race/compensating revert uses **`:rollback`** suffix on inverse delta. Idempotent replay checks **`TryGet`** before apply. If grants succeed but **`TryRecord`** returns `false` (concurrent race), this call **rolls back** item grants, reverts skill XP via **`SkillProgressionGrantOperations.TryRevertAppliedSkillGrants`**, and reverts reputation via inverse **`TryApplyDelta`** — then returns the existing store event (mirror **`EncounterCompletionOperations`** compensating rollback when **`TryMarkCompleted`** loses). **Quest-state wiring (NEO-128):** **`QuestStateOperations.TryMarkComplete`** and **`QuestObjectiveWiring`** terminal-step completion call **`TryDeliverQuestCompletion`** before **`IPlayerQuestStateStore.TryMarkComplete`**. **Contract wiring (NEO-149):** **`ContractCompletionOperations.TryCompleteOnEncounterClear`** — invoked from **`EncounterCompletionOperations`** after encounter loot + quest wiring; deliver-then-mark via **`TryDeliverContractCompletion`**, outcome append via **`IContractOutcomeStore`**. Bruno quest-flow smoke: `bruno/neon-sprawl-server/quest-progress/Accept grid contract after operator chain.bru` (NEO-138). Plans: [NEO-127](../../docs/plans/NEO-127-implementation-plan.md), [NEO-128](../../docs/plans/NEO-128-implementation-plan.md), [NEO-138](../../docs/plans/NEO-138-implementation-plan.md), [NEO-148](../../docs/plans/NEO-148-implementation-plan.md), [NEO-149](../../docs/plans/NEO-149-implementation-plan.md). ### Reward telemetry hooks (NEO-130) From d1df9400846608d1b39ea6dfe3841ec59bb230d8 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 27 Jun 2026 21:50:13 -0400 Subject: [PATCH 3/7] NEO-149: add code review for contract completion wiring --- docs/reviews/2026-06-27-NEO-149.md | 54 ++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 docs/reviews/2026-06-27-NEO-149.md diff --git a/docs/reviews/2026-06-27-NEO-149.md b/docs/reviews/2026-06-27-NEO-149.md new file mode 100644 index 0000000..0f74652 --- /dev/null +++ b/docs/reviews/2026-06-27-NEO-149.md @@ -0,0 +1,54 @@ +# Code review — NEO-149 contract completion encounter wiring + +**Date:** 2026-06-27 +**Scope:** Branch `NEO-149-e7m4-06-contract-completion-encounter-wiring` — commits `8d9d02f`..`bcf6fd2` vs `origin/main` +**Base:** `origin/main` + +## Verdict + +**Approve with nits** + +## Summary + +NEO-149 adds `ContractCompletionOperations.TryCompleteOnEncounterClear` — deliver-then-mark contract completion on encounter clear, following the NEO-128 quest precedent and NEO-148 router. The orchestrator resolves the active instance, matches `encounterTemplateId`, calls `TryDeliverContractCompletion`, marks the instance complete, and appends an outcome audit row. Encounter wiring invokes the hook after encounter loot and quest objective wiring; contract payout failure does not block encounter success (result discarded). Contract store/registry dependencies are plumbed through `EncounterCombatWiring` and `AbilityCastApi`. `QuestFixtureRequest.resetContractInstanceIds` clears instance, delivery, and outcome rows for Bruno/dev reset loops. Five unit tests, one DI integration test, and one encounter-completion wiring test cover match/mismatch, delivery deny, idempotent replay, and combined encounter + contract loot. Implementation plan reconciliation, `server/README.md`, and E7.M4 module anchor are updated. Server-only slice; client work explicitly deferred to NEO-153/NEO-154 after NEO-151 HTTP. Full suite: **904** tests passed locally. + +## Documentation checked + +| Path | Result | +|------|--------| +| `docs/plans/NEO-149-implementation-plan.md` | **Matches** — AC checklist complete; shipped reconciliation aligns with diff | +| `docs/plans/E7M4-pre-production-backlog.md` (E7M4-06) | **Matches** — orchestrator + encounter wiring in scope; HTTP Bruno loop deferred to NEO-151 | +| `docs/decomposition/modules/E7_M4_ContractMissionGenerator.md` | **Matches** — NEO-149 completion wiring anchor added | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Partially matches** — E7.M4 row includes NEO-144–NEO-148 but not **E7M4-06 / NEO-149** yet | +| `server/README.md` | **Matches** — contract completion operations section, wiring order, reason codes, fixture reset | +| Client counterpart (NEO-153, NEO-154) | **N/A** — explicitly out of scope; no prototype-complete claim | + +Register/tracking table should be updated for E7.M4 (NEO-149) after merge. + +## Blocking issues + +None. + +## Suggestions + +1. **Update implementation alignment register** — Add **E7M4-06 / NEO-149 landed** to the E7.M4 row in `docs/decomposition/modules/documentation_and_implementation_alignment.md` (orchestrator + encounter wiring + quest-fixture reset), consistent with NEO-144–NEO-148 entries. + +2. **Quest-fixture test for contract reset** — `QuestFixtureOperations` now clears contract instance, delivery, and outcome rows via `resetContractInstanceIds`, but `QuestFixtureApiTests` has no parallel to `PostQuestFixture_ShouldClearRewardDeliveryAndAudit_WhenResetQuestIdsProvided`. A test that issues a contract, completes it, resets via fixture, and asserts stores are empty would lock the NEO-148 follow-on reset path. + +3. **`TryMarkComplete` failure reason code** — When delivery succeeds but `TryMarkComplete` fails without leaving a completed snapshot, the orchestrator returns `invalid_ids` (line 122 in `ContractCompletionOperations.cs`). `QuestStateOperations.TryMarkComplete` uses richer fallback codes (`NotActive`, `UnknownPlayer`). Consider aligning deny semantics in a follow-on if store-failure paths become testable; not blocking for prototype (same deliver-then-mark stuck-state risk as quest path when mark loses after delivery). + +## Nits + +- Nit: Idempotent replay after successful completion returns `no_active_contract` (instance no longer active) rather than hitting the defensive `completed` snapshot branch at lines 59–61. Behavior is correct — no double grant — but the reason code differs from the plan’s “completed replay” wording. +- Nit: `TryCompleteOnEncounterClear_ShouldBeIdempotent_WhenReplayed` performs success assertions on the first completion inside **Arrange**; prefer moving first-call verification to **Assert** after **Act** for strict AAA, or extract a shared setup helper. +- Nit: Plan lists optional `EncounterCombatWiring` end-to-end issue → cast → contract completed; only compile-fix call-site updates landed. Acceptable given integration + encounter completion tests cover the path. +- Nit: Bruno `Reset contract instance via quest fixture.bru` uses placeholder `ci_000…` id — fine until NEO-151 wires issue → clear → GET; helper `resetContractInstances` in `bruno-dev-fixture-helper.js` is the useful piece for that story. + +## Verification + +```bash +dotnet test NeonSprawl.sln +dotnet test NeonSprawl.sln --filter "FullyQualifiedName~ContractCompletionOperations|FullyQualifiedName~EncounterCompletionOperationsTests.TryCompleteAndGrant_ShouldApplyContractBundle" +``` + +Manual Godot QA: none (server engine per plan). Bruno issue → clear → GET loop: NEO-151. From 68ba26724bed1d87499490a45dbab6aaeb251103 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 27 Jun 2026 21:51:17 -0400 Subject: [PATCH 4/7] NEO-149: address code review follow-ups Update E7.M4 alignment register, add quest-fixture contract reset API test, align mark-complete deny codes with instance reason codes, refactor idempotent replay test AAA, strike review findings done. --- ...umentation_and_implementation_alignment.md | 2 +- docs/reviews/2026-06-27-NEO-149.md | 12 +-- .../ContractCompletionOperationsTests.cs | 29 ++++--- .../Game/Quests/QuestFixtureApiTests.cs | 84 +++++++++++++++++++ .../Contracts/ContractCompletionOperations.cs | 15 +++- 5 files changed, 121 insertions(+), 21 deletions(-) diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index b04993b..fe72db9 100644 --- a/docs/decomposition/modules/documentation_and_implementation_alignment.md +++ b/docs/decomposition/modules/documentation_and_implementation_alignment.md @@ -62,7 +62,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not | E7.M2 | Ready | **E7M2-01 catalog landed ([NEO-124](https://linear.app/neon-sprawl/issue/NEO-124)):** `quest-reward-bundle` + `quest-skill-xp-grant` schemas; **`completionRewardBundle`** on four frozen E7.M1 quests; CI bundle freeze + cross-ref gates. **E7M2-02 server load ([NEO-125](https://linear.app/neon-sprawl/issue/NEO-125)):** fail-fast quest catalog **`completionRewardBundle`** validation at startup — `PrototypeE7M2QuestCatalogRules` + bundle schema `$ref` registration ([NEO-125](../../plans/NEO-125-implementation-plan.md)); [server README — Quest catalog (NEO-125)](../../../server/README.md#quest-catalog-contentquests-neo-113). **E7M2-03 delivery store ([NEO-126](https://linear.app/neon-sprawl/issue/NEO-126)):** idempotent **`IRewardDeliveryStore`** + **`RewardDeliveryEvent`** in `Game/Rewards/` — in-memory prototype ([NEO-126](../../plans/NEO-126-implementation-plan.md)); [server README — Reward delivery store (NEO-126)](../../../server/README.md#reward-delivery-store-neo-126). **E7M2-04 router apply ([NEO-127](https://linear.app/neon-sprawl/issue/NEO-127)):** **`RewardRouterOperations.TryDeliverQuestCompletion`** — item + skill XP bundle apply with compensating rollback + store record ([NEO-127](../../plans/NEO-127-implementation-plan.md)); [server README — Reward router (NEO-127)](../../../server/README.md#reward-router-neo-127). **E7M2-05 quest-state wiring ([NEO-128](https://linear.app/neon-sprawl/issue/NEO-128)):** **`QuestStateOperations.TryMarkComplete`** + **`QuestObjectiveWiring`** deliver-then-mark via router ([NEO-128](../../plans/NEO-128-implementation-plan.md)); [server README — Quest state operations (NEO-117)](../../../server/README.md#quest-state-operations-neo-117). **E7M2-06 HTTP read ([NEO-129](https://linear.app/neon-sprawl/issue/NEO-129)):** **`GET …/quest-progress`** **`completionRewardSummary`** from **`IRewardDeliveryStore`** ([NEO-129](../../plans/NEO-129-implementation-plan.md)); Bruno `bruno/neon-sprawl-server/quest-progress/Get quest progress after gather intro complete.bru`. **E7M2-07 telemetry ([NEO-130](https://linear.app/neon-sprawl/issue/NEO-130)):** comment-only **`reward_delivery`** + **`unlock_granted`** stub hook sites in **`RewardRouterOperations.TryDeliverQuestCompletion`** ([NEO-130](../../plans/NEO-130-implementation-plan.md)); [server README — Reward telemetry hooks (NEO-130)](../../../server/README.md#reward-telemetry-hooks-neo-130). **E7M2-08 client HUD ([NEO-131](https://linear.app/neon-sprawl/issue/NEO-131)):** Godot **`QuestRewardDeliveryLabel`** paints **`completionRewardSummary`** on in-session completion transition ([NEO-131](../../plans/NEO-131-implementation-plan.md)); [client README — Quest completion reward HUD (NEO-131)](../../../client/README.md#quest-completion-reward-hud-neo-131). **E7M2-09 client capstone ([NEO-132](https://linear.app/neon-sprawl/issue/NEO-132)):** playable quest reward delivery loop — [`NEO-132` manual QA](../../manual-qa/NEO-132.md); [client README — End-to-end quest reward loop (NEO-132)](../../../client/README.md#end-to-end-quest-reward-loop-neo-132); plan [NEO-132](../../plans/NEO-132-implementation-plan.md). **Epic 7 Slice 2 client capstone complete.** Backlog **E7M2-01** [NEO-124](https://linear.app/neon-sprawl/issue/NEO-124) → **E7M2-09** [NEO-132](https://linear.app/neon-sprawl/issue/NEO-132) **landed**. **NEO-43 prep landed:** **`MissionRewardSkillXpGrant`**. **Encounter loot unchanged** (E5.M3 direct grants). Upstream: E7.M1 **Ready**, E2.M2 grant stack, E3.M3 inventory **Ready**. | [E7M2-prototype-backlog.md](../../plans/E7M2-prototype-backlog.md), [E7_M2](E7_M2_RewardAndUnlockRouter.md), [NEO-124](../../plans/NEO-124-implementation-plan.md), [NEO-125](../../plans/NEO-125-implementation-plan.md), [NEO-126](../../plans/NEO-126-implementation-plan.md), [NEO-127](../../plans/NEO-127-implementation-plan.md), [NEO-128](../../plans/NEO-128-implementation-plan.md), [NEO-129](../../plans/NEO-129-implementation-plan.md), [NEO-130](../../plans/NEO-130-implementation-plan.md), [NEO-131](../../plans/NEO-131-implementation-plan.md), [NEO-132](../../plans/NEO-132-implementation-plan.md), [NEO-43](../../plans/NEO-43-implementation-plan.md), label **`E7.M2`** on NEO-124–NEO-132 | | E7.M1 | Ready | **E7M1-01 catalog landed ([NEO-112](https://linear.app/neon-sprawl/issue/NEO-112)):** quest schemas, `prototype_quests.json`, CI gates (four frozen quest ids, objective cross-refs, acyclic prerequisites, chain terminal token). **E7M1-02 server load ([NEO-113](https://linear.app/neon-sprawl/issue/NEO-113)):** fail-fast startup load of `content/quests/*_quests.json` — `server/NeonSprawl.Server/Game/Quests/` ([NEO-113](../../plans/NEO-113-implementation-plan.md)); [server README — Quest catalog](../../../server/README.md#quest-catalog-contentquests-neo-113). **E7M1-03 registry ([NEO-114](https://linear.app/neon-sprawl/issue/NEO-114)):** **`IQuestDefinitionRegistry`** + DI ([NEO-114](../../plans/NEO-114-implementation-plan.md)). **E7M1-04 HTTP read ([NEO-115](https://linear.app/neon-sprawl/issue/NEO-115)):** **`GET /game/world/quest-definitions`** — `QuestDefinitionsWorldApi` + DTOs ([NEO-115](../../plans/NEO-115-implementation-plan.md)); [server README — Quest definitions (NEO-115)](../../../server/README.md#quest-definitions-neo-115); Bruno `bruno/neon-sprawl-server/quest-definitions/`. **E7M1-05 store ([NEO-116](https://linear.app/neon-sprawl/issue/NEO-116)):** **`IPlayerQuestStateStore`** + in-memory/Postgres persistence ([NEO-116](../../plans/NEO-116-implementation-plan.md)); [server README — Quest progress store (NEO-116)](../../../server/README.md#quest-progress-store-neo-116). **E7M1-06 operations ([NEO-117](https://linear.app/neon-sprawl/issue/NEO-117)):** **`QuestStateOperations`** + reason codes ([NEO-117](../../plans/NEO-117-implementation-plan.md)); [server README — Quest state operations (NEO-117)](../../../server/README.md#quest-state-operations-neo-117). **E7M1-07 objective wiring ([NEO-118](https://linear.app/neon-sprawl/issue/NEO-118)):** **`QuestObjectiveWiring`** on gather/craft/encounter + **`inventory_has_item`** snapshot passes ([NEO-118](../../plans/NEO-118-implementation-plan.md)); [server README — Quest objective wiring (NEO-118)](../../../server/README.md#quest-objective-wiring-neo-118). **E7M1-08 per-player GET ([NEO-119](https://linear.app/neon-sprawl/issue/NEO-119)):** **`GET /game/players/{id}/quest-progress`** — `QuestProgressApi` + DTOs; GET-side inventory refresh ([NEO-119](../../plans/NEO-119-implementation-plan.md)); [server README — Per-player quest progress (NEO-119)](../../../server/README.md#per-player-quest-progress-neo-119); Bruno `bruno/neon-sprawl-server/quest-progress/`. **E7M1-09 accept POST ([NEO-120](https://linear.app/neon-sprawl/issue/NEO-120)):** **`POST /game/players/{id}/quests/{questId}/accept`** — `QuestAcceptApi` + DTOs; **`QuestStateOperations.TryAccept`** ([NEO-120](../../plans/NEO-120-implementation-plan.md)); [server README — Quest accept POST (NEO-120)](../../../server/README.md#quest-accept-post-neo-120); Bruno accept spine in `bruno/neon-sprawl-server/quest-progress/`. **E7M1-10 telemetry ([NEO-121](https://linear.app/neon-sprawl/issue/NEO-121)):** comment-only **`quest_start`**, **`quest_step_complete`**, **`quest_complete`**, **`quest_accept_denied`** hook sites in **`QuestStateOperations`** ([NEO-121](../../plans/NEO-121-implementation-plan.md)); [server README — Quest telemetry hooks (NEO-121)](../../../server/README.md#quest-telemetry-hooks-neo-121). **E7M1-11 client HUD ([NEO-122](https://linear.app/neon-sprawl/issue/NEO-122)):** **`quest_progress_client.gd`**, **`quest_definitions_client.gd`**, **`QuestProgressLabel`** / **`QuestAcceptFeedbackLabel`**; boot + event-driven GET refresh; **Q** / **Shift+Q** accept ([NEO-122](../../plans/NEO-122-implementation-plan.md), [`NEO-122` manual QA](../../manual-qa/NEO-122.md)); [client README — Quest progress + accept HUD (NEO-122)](../../../client/README.md#quest-progress--accept-hud-neo-122). **E7M1-12 client capstone ([NEO-123](https://linear.app/neon-sprawl/issue/NEO-123)):** playable four-quest onboarding chain — [`NEO-123` manual QA](../../manual-qa/NEO-123.md); [client README — End-to-end onboarding quest loop (NEO-123)](../../../client/README.md#end-to-end-onboarding-quest-loop-neo-123); plan [NEO-123](../../plans/NEO-123-implementation-plan.md). **Epic 7 Slice 1 client capstone complete.** Backlog **E7M1-01** [NEO-112](https://linear.app/neon-sprawl/issue/NEO-112) → **E7M1-12** [NEO-123](https://linear.app/neon-sprawl/issue/NEO-123) **landed**. Upstream: E3.M1 gather **Ready**, E3.M2 craft **Ready**, E5.M3 encounter **`contract_handoff_token`** + **`EncounterCompleteEvent`** **Ready**. | [NEO-123 plan](../../plans/NEO-123-implementation-plan.md), [NEO-122 plan](../../plans/NEO-122-implementation-plan.md), [NEO-121 plan](../../plans/NEO-121-implementation-plan.md), [NEO-120 plan](../../plans/NEO-120-implementation-plan.md), [NEO-119 plan](../../plans/NEO-119-implementation-plan.md), [NEO-118 plan](../../plans/NEO-118-implementation-plan.md), [NEO-117 plan](../../plans/NEO-117-implementation-plan.md), [NEO-116 plan](../../plans/NEO-116-implementation-plan.md), [NEO-115 plan](../../plans/NEO-115-implementation-plan.md), [NEO-114 plan](../../plans/NEO-114-implementation-plan.md), [NEO-113 plan](../../plans/NEO-113-implementation-plan.md), [NEO-112 plan](../../plans/NEO-112-implementation-plan.md), [E7M1-prototype-backlog.md](../../plans/E7M1-prototype-backlog.md), [E7_M1](E7_M1_QuestStateMachine.md), label **`E7.M1`** on NEO-112–NEO-123 | | E7.M3 | Ready | **E7M3-01 catalog landed ([NEO-133](https://linear.app/neon-sprawl/issue/NEO-133)):** faction + gate/rep schemas, `prototype_factions.json`, quest extensions, five-quest CI gates, minimal server roster parity. **E7M3-02 server load landed ([NEO-134](https://linear.app/neon-sprawl/issue/NEO-134)):** fail-fast faction catalog, `IFactionDefinitionRegistry`, quest `factionGateRules` / `reputationGrants` parse + E7M3 cross-ref/bundle/grid gates ([NEO-134 plan](../../plans/NEO-134-implementation-plan.md)). **E7M3-03 stores landed ([NEO-135](https://linear.app/neon-sprawl/issue/NEO-135)):** `IFactionStandingStore`, `IReputationDeltaStore`, in-memory + Postgres `V009`/`V010` ([NEO-135 plan](../../plans/NEO-135-implementation-plan.md)); [server README — Faction standing store (NEO-135)](../../../server/README.md#faction-standing-store-neo-135). **E7M3-04 ops landed ([NEO-136](https://linear.app/neon-sprawl/issue/NEO-136)):** `ReputationOperations.TryApplyDelta` — standing + audit orchestration with compensating rollback ([NEO-136 plan](../../plans/NEO-136-implementation-plan.md)); [server README — ReputationOperations (NEO-136)](../../../server/README.md#reputationoperations-neo-136). **E7M3-05 gate eval landed ([NEO-137](https://linear.app/neon-sprawl/issue/NEO-137)):** `FactionGateOperations.TryEvaluate` wired into `QuestStateOperations.TryAccept`; `faction_gate_blocked` deny ([NEO-137 plan](../../plans/NEO-137-implementation-plan.md)); [server README — FactionGateOperations (NEO-137)](../../../server/README.md#factiongateoperations-neo-137). **E7M3-06 reward router landed ([NEO-138](https://linear.app/neon-sprawl/issue/NEO-138)):** `RewardRouterOperations` applies `reputationGrants` via `ReputationOperations`; Bruno grid-contract accept success after organic operator chain ([NEO-138 plan](../../plans/NEO-138-implementation-plan.md)); [server README — Reward router (NEO-127)](../../../server/README.md#reward-router-neo-127). **E7M3-07 HTTP read landed ([NEO-139](https://linear.app/neon-sprawl/issue/NEO-139)):** **`GET …/faction-standing`** — `FactionStandingApi` + DTOs; Bruno `bruno/neon-sprawl-server/faction-standing/` ([NEO-139 plan](../../plans/NEO-139-implementation-plan.md)); [server README — Faction standing read (NEO-139)](../../../server/README.md#faction-standing-read-neo-139). **E7M3-08 quest HTTP projections landed ([NEO-140](https://linear.app/neon-sprawl/issue/NEO-140)):** world GET **`factionGateRules`** + quest-progress **`completionRewardSummary.reputationGrants`**; Bruno quest-definitions + operator-chain progress bru ([NEO-140 plan](../../plans/NEO-140-implementation-plan.md)); [server README — Quest definitions (NEO-115)](../../../server/README.md#quest-definitions-neo-115), [Per-player quest progress (NEO-119)](../../../server/README.md#per-player-quest-progress-neo-119). **E7M3-09 telemetry landed ([NEO-141](https://linear.app/neon-sprawl/issue/NEO-141)):** comment-only **`reputation_delta`** + **`faction_gate_blocked`** hook sites in ops layers ([NEO-141 plan](../../plans/NEO-141-implementation-plan.md)); [server README — Faction telemetry hooks (NEO-141)](../../../server/README.md#faction-telemetry-hooks-neo-141). **E7M3-10 client faction HUD landed ([NEO-142](https://linear.app/neon-sprawl/issue/NEO-142)):** `faction_standing_client.gd`, standing label + gate deny + rep reward lines ([NEO-142 plan](../../plans/NEO-142-implementation-plan.md)); [client README — Faction standing + gate feedback HUD (NEO-142)](../../../client/README.md#faction-standing--gate-feedback-hud-neo-142); [`NEO-142` manual QA](../../manual-qa/NEO-142.md). **E7M3-11 client capstone landed ([NEO-143](https://linear.app/neon-sprawl/issue/NEO-143)):** playable faction reputation + gate capstone — [`NEO-143` manual QA](../../manual-qa/NEO-143.md); [client README — End-to-end faction reputation loop (NEO-143)](../../../client/README.md#end-to-end-faction-reputation-loop-neo-143); plan [NEO-143](../../plans/NEO-143-implementation-plan.md). **Epic 7 Slice 3 client capstone complete.** Two frozen factions; operator-chain **`reputationGrants`** (+15 Grid Operators); gated **`prototype_quest_grid_contract`**. Upstream: E7.M1 **Ready**, E7.M2 **Ready**. | [NEO-133 plan](../../plans/NEO-133-implementation-plan.md), [NEO-134 plan](../../plans/NEO-134-implementation-plan.md), [NEO-135 plan](../../plans/NEO-135-implementation-plan.md), [NEO-136 plan](../../plans/NEO-136-implementation-plan.md), [NEO-137 plan](../../plans/NEO-137-implementation-plan.md), [NEO-138 plan](../../plans/NEO-138-implementation-plan.md), [NEO-139 plan](../../plans/NEO-139-implementation-plan.md), [NEO-140 plan](../../plans/NEO-140-implementation-plan.md), [NEO-141 plan](../../plans/NEO-141-implementation-plan.md), [NEO-142 plan](../../plans/NEO-142-implementation-plan.md), [NEO-143 plan](../../plans/NEO-143-implementation-plan.md), [E7M3-pre-production-backlog.md](../../plans/E7M3-pre-production-backlog.md), [E7_M3](E7_M3_FactionReputationLedger.md), label **`E7.M3`** on NEO-133–NEO-143 | -| E7.M4 | In Progress | **E7M4-01 catalog landed ([NEO-144](https://linear.app/neon-sprawl/issue/NEO-144)):** `contract-template` + `contract-seed` schemas; `prototype_contract_templates.json`; CI roster/freeze/cross-ref/band-cap gates. **E7M4-02 server load landed ([NEO-145](https://linear.app/neon-sprawl/issue/NEO-145)):** fail-fast contract template catalog, `IContractTemplateRegistry`, `PrototypeE7M4ContractCatalogRules` at startup ([NEO-145 plan](../../plans/NEO-145-implementation-plan.md)); [server README — Contract template catalog](../../../server/README.md#contract-template-catalog-contentcontracts-neo-145). **E7M4-03 stores landed ([NEO-146](https://linear.app/neon-sprawl/issue/NEO-146)):** `IContractInstanceStore`, `IContractOutcomeStore`, in-memory + Postgres `V011`/`V012` ([NEO-146 plan](../../plans/NEO-146-implementation-plan.md)); [server README — Contract instance store (NEO-146)](../../../server/README.md#contract-instance-store-neo-146), [Contract outcome store (NEO-146)](../../../server/README.md#contract-outcome-store-neo-146); Bruno `bruno/neon-sprawl-server/contract-stores/`. **E7M4-04 generator landed ([NEO-147](https://linear.app/neon-sprawl/issue/NEO-147)):** `ContractGeneratorOperations.TryIssue`, `ContractGeneratorReasonCodes`, deterministic `ci_{hex}` instance ids ([NEO-147 plan](../../plans/NEO-147-implementation-plan.md)); [server README — Contract generator operations (NEO-147)](../../../server/README.md#contract-generator-operations-neo-147). **E7M4-05 reward router landed ([NEO-148](https://linear.app/neon-sprawl/issue/NEO-148)):** `RewardRouterOperations.TryDeliverContractCompletion`, source-kind-aware `IRewardDeliveryStore` / `RewardDeliveryEvent` extension ([NEO-148 plan](../../plans/NEO-148-implementation-plan.md)); [server README — Reward router (NEO-127)](../../../server/README.md#reward-router-neo-127), [Reward delivery store (NEO-126)](../../../server/README.md#reward-delivery-store-neo-126). **Decomposition landed:** Epic 7 Slice 4 backlog [E7M4-pre-production-backlog.md](../../plans/E7M4-pre-production-backlog.md) — **E7M4-01** [NEO-144](https://linear.app/neon-sprawl/issue/NEO-144) → **E7M4-11** [NEO-154](https://linear.app/neon-sprawl/issue/NEO-154); label **`E7.M4`**. Prototype spine: **`prototype_contract_clear_combat_pocket`** template → **`prototype_combat_pocket`** encounter clear → repeat payout **`scrap_metal_bulk` ×5** + salvage **15**; one active contract per player; economy band caps at issue; E4.M1 live zone deferred (content **`zoneDifficultyBand`**). Client capstone **E7M4-11** [NEO-154](https://linear.app/neon-sprawl/issue/NEO-154). Upstream: E5.M3 **Ready**, E7.M2 router **Ready**, E7.M3 standing **Ready**. | [E7M4-pre-production-backlog.md](../../plans/E7M4-pre-production-backlog.md), [E7_M4](E7_M4_ContractMissionGenerator.md), [NEO-144](../../plans/NEO-144-implementation-plan.md), [NEO-145](../../plans/NEO-145-implementation-plan.md), [NEO-146](../../plans/NEO-146-implementation-plan.md), [NEO-147](../../plans/NEO-147-implementation-plan.md), [NEO-148](../../plans/NEO-148-implementation-plan.md), label **`E7.M4`** on NEO-144–NEO-154 | +| E7.M4 | In Progress | **E7M4-01 catalog landed ([NEO-144](https://linear.app/neon-sprawl/issue/NEO-144)):** `contract-template` + `contract-seed` schemas; `prototype_contract_templates.json`; CI roster/freeze/cross-ref/band-cap gates. **E7M4-02 server load landed ([NEO-145](https://linear.app/neon-sprawl/issue/NEO-145)):** fail-fast contract template catalog, `IContractTemplateRegistry`, `PrototypeE7M4ContractCatalogRules` at startup ([NEO-145 plan](../../plans/NEO-145-implementation-plan.md)); [server README — Contract template catalog](../../../server/README.md#contract-template-catalog-contentcontracts-neo-145). **E7M4-03 stores landed ([NEO-146](https://linear.app/neon-sprawl/issue/NEO-146)):** `IContractInstanceStore`, `IContractOutcomeStore`, in-memory + Postgres `V011`/`V012` ([NEO-146 plan](../../plans/NEO-146-implementation-plan.md)); [server README — Contract instance store (NEO-146)](../../../server/README.md#contract-instance-store-neo-146), [Contract outcome store (NEO-146)](../../../server/README.md#contract-outcome-store-neo-146); Bruno `bruno/neon-sprawl-server/contract-stores/`. **E7M4-04 generator landed ([NEO-147](https://linear.app/neon-sprawl/issue/NEO-147)):** `ContractGeneratorOperations.TryIssue`, `ContractGeneratorReasonCodes`, deterministic `ci_{hex}` instance ids ([NEO-147 plan](../../plans/NEO-147-implementation-plan.md)); [server README — Contract generator operations (NEO-147)](../../../server/README.md#contract-generator-operations-neo-147). **E7M4-05 reward router landed ([NEO-148](https://linear.app/neon-sprawl/issue/NEO-148)):** `RewardRouterOperations.TryDeliverContractCompletion`, source-kind-aware `IRewardDeliveryStore` / `RewardDeliveryEvent` extension ([NEO-148 plan](../../plans/NEO-148-implementation-plan.md)); [server README — Reward router (NEO-127)](../../../server/README.md#reward-router-neo-127), [Reward delivery store (NEO-126)](../../../server/README.md#reward-delivery-store-neo-126). **E7M4-06 completion wiring landed ([NEO-149](https://linear.app/neon-sprawl/issue/NEO-149)):** `ContractCompletionOperations.TryCompleteOnEncounterClear`, encounter hook after E5.M3 loot + quest wiring, `QuestFixtureRequest.resetContractInstanceIds` ([NEO-149 plan](../../plans/NEO-149-implementation-plan.md)); [server README — Contract completion operations (NEO-149)](../../../server/README.md#contract-completion-operations-neo-149); Bruno `bruno/neon-sprawl-server/contract-stores/Reset contract instance via quest fixture.bru`. **Decomposition landed:** Epic 7 Slice 4 backlog [E7M4-pre-production-backlog.md](../../plans/E7M4-pre-production-backlog.md) — **E7M4-01** [NEO-144](https://linear.app/neon-sprawl/issue/NEO-144) → **E7M4-11** [NEO-154](https://linear.app/neon-sprawl/issue/NEO-154); label **`E7.M4`**. Prototype spine: **`prototype_contract_clear_combat_pocket`** template → **`prototype_combat_pocket`** encounter clear → repeat payout **`scrap_metal_bulk` ×5** + salvage **15**; one active contract per player; economy band caps at issue; E4.M1 live zone deferred (content **`zoneDifficultyBand`**). Client capstone **E7M4-11** [NEO-154](https://linear.app/neon-sprawl/issue/NEO-154). Upstream: E5.M3 **Ready**, E7.M2 router **Ready**, E7.M3 standing **Ready**. | [E7M4-pre-production-backlog.md](../../plans/E7M4-pre-production-backlog.md), [E7_M4](E7_M4_ContractMissionGenerator.md), [NEO-144](../../plans/NEO-144-implementation-plan.md), [NEO-145](../../plans/NEO-145-implementation-plan.md), [NEO-146](../../plans/NEO-146-implementation-plan.md), [NEO-147](../../plans/NEO-147-implementation-plan.md), [NEO-148](../../plans/NEO-148-implementation-plan.md), [NEO-149](../../plans/NEO-149-implementation-plan.md), label **`E7.M4`** on NEO-144–NEO-154 | --- diff --git a/docs/reviews/2026-06-27-NEO-149.md b/docs/reviews/2026-06-27-NEO-149.md index 0f74652..06fd154 100644 --- a/docs/reviews/2026-06-27-NEO-149.md +++ b/docs/reviews/2026-06-27-NEO-149.md @@ -19,11 +19,11 @@ NEO-149 adds `ContractCompletionOperations.TryCompleteOnEncounterClear` — deli | `docs/plans/NEO-149-implementation-plan.md` | **Matches** — AC checklist complete; shipped reconciliation aligns with diff | | `docs/plans/E7M4-pre-production-backlog.md` (E7M4-06) | **Matches** — orchestrator + encounter wiring in scope; HTTP Bruno loop deferred to NEO-151 | | `docs/decomposition/modules/E7_M4_ContractMissionGenerator.md` | **Matches** — NEO-149 completion wiring anchor added | -| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Partially matches** — E7.M4 row includes NEO-144–NEO-148 but not **E7M4-06 / NEO-149** yet | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E7.M4 row includes **E7M4-06 / NEO-149** (orchestrator + encounter wiring + quest-fixture reset) | | `server/README.md` | **Matches** — contract completion operations section, wiring order, reason codes, fixture reset | | Client counterpart (NEO-153, NEO-154) | **N/A** — explicitly out of scope; no prototype-complete claim | -Register/tracking table should be updated for E7.M4 (NEO-149) after merge. +Register/tracking table updated for E7.M4 (NEO-149). ## Blocking issues @@ -31,16 +31,16 @@ None. ## Suggestions -1. **Update implementation alignment register** — Add **E7M4-06 / NEO-149 landed** to the E7.M4 row in `docs/decomposition/modules/documentation_and_implementation_alignment.md` (orchestrator + encounter wiring + quest-fixture reset), consistent with NEO-144–NEO-148 entries. +1. ~~**Update implementation alignment register** — Add **E7M4-06 / NEO-149 landed** to the E7.M4 row in `docs/decomposition/modules/documentation_and_implementation_alignment.md` (orchestrator + encounter wiring + quest-fixture reset), consistent with NEO-144–NEO-148 entries.~~ **Done.** E7.M4 row + plan link updated. -2. **Quest-fixture test for contract reset** — `QuestFixtureOperations` now clears contract instance, delivery, and outcome rows via `resetContractInstanceIds`, but `QuestFixtureApiTests` has no parallel to `PostQuestFixture_ShouldClearRewardDeliveryAndAudit_WhenResetQuestIdsProvided`. A test that issues a contract, completes it, resets via fixture, and asserts stores are empty would lock the NEO-148 follow-on reset path. +2. ~~**Quest-fixture test for contract reset** — `QuestFixtureOperations` now clears contract instance, delivery, and outcome rows via `resetContractInstanceIds`, but `QuestFixtureApiTests` has no parallel to `PostQuestFixture_ShouldClearRewardDeliveryAndAudit_WhenResetQuestIdsProvided`. A test that issues a contract, completes it, resets via fixture, and asserts stores are empty would lock the NEO-148 follow-on reset path.~~ **Done.** `PostQuestFixture_ShouldClearContractStores_WhenResetContractInstanceIdsProvided`. -3. **`TryMarkComplete` failure reason code** — When delivery succeeds but `TryMarkComplete` fails without leaving a completed snapshot, the orchestrator returns `invalid_ids` (line 122 in `ContractCompletionOperations.cs`). `QuestStateOperations.TryMarkComplete` uses richer fallback codes (`NotActive`, `UnknownPlayer`). Consider aligning deny semantics in a follow-on if store-failure paths become testable; not blocking for prototype (same deliver-then-mark stuck-state risk as quest path when mark loses after delivery). +3. ~~**`TryMarkComplete` failure reason code** — When delivery succeeds but `TryMarkComplete` fails without leaving a completed snapshot, the orchestrator returns `invalid_ids` (line 122 in `ContractCompletionOperations.cs`). `QuestStateOperations.TryMarkComplete` uses richer fallback codes (`NotActive`, `UnknownPlayer`). Consider aligning deny semantics in a follow-on if store-failure paths become testable; not blocking for prototype (same deliver-then-mark stuck-state risk as quest path when mark loses after delivery).~~ **Done.** Final deny uses `player_not_writable`, `instance_not_active`, or `instance_not_found` instead of `invalid_ids`. ## Nits - Nit: Idempotent replay after successful completion returns `no_active_contract` (instance no longer active) rather than hitting the defensive `completed` snapshot branch at lines 59–61. Behavior is correct — no double grant — but the reason code differs from the plan’s “completed replay” wording. -- Nit: `TryCompleteOnEncounterClear_ShouldBeIdempotent_WhenReplayed` performs success assertions on the first completion inside **Arrange**; prefer moving first-call verification to **Assert** after **Act** for strict AAA, or extract a shared setup helper. +- ~~Nit: `TryCompleteOnEncounterClear_ShouldBeIdempotent_WhenReplayed` performs success assertions on the first completion inside **Arrange**; prefer moving first-call verification to **Assert** after **Act** for strict AAA, or extract a shared setup helper.~~ **Done.** `IssueAndCompleteOnce` setup helper; replay test Act/Assert only. - Nit: Plan lists optional `EncounterCombatWiring` end-to-end issue → cast → contract completed; only compile-fix call-site updates landed. Acceptable given integration + encounter completion tests cover the path. - Nit: Bruno `Reset contract instance via quest fixture.bru` uses placeholder `ci_000…` id — fine until NEO-151 wires issue → clear → GET; helper `resetContractInstances` in `bruno-dev-fixture-helper.js` is the useful piece for that story. diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractCompletionOperationsTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractCompletionOperationsTests.cs index 4c63022..17048e4 100644 --- a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractCompletionOperationsTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractCompletionOperationsTests.cs @@ -131,12 +131,7 @@ public sealed class ContractCompletionOperationsTests // Arrange await using var factory = new InMemoryWebApplicationFactory(); var deps = ResolveDependencies(factory); - var issued = IssuePrototypeContract(deps); - var instanceId = issued.Snapshot!.ContractInstanceId; - var first = CompleteOnEncounterClear(deps, EncounterId); - Assert.True(first.Success); - deps.InventoryStore.TryGetSnapshot(PlayerId, out var afterFirst); - var salvageAfterFirst = deps.SkillProgressionStore.GetXpTotals(PlayerId).GetValueOrDefault("salvage"); + var instanceId = IssueAndCompleteOnce(deps); // Act var replay = CompleteOnEncounterClear(deps, EncounterId); @@ -144,19 +139,27 @@ public sealed class ContractCompletionOperationsTests // Assert Assert.True(replay.Success); Assert.Equal(ContractCompletionReasonCodes.NoActiveContract, replay.ReasonCode); - deps.InventoryStore.TryGetSnapshot(PlayerId, out var afterReplay); - Assert.Equal( - CountBagItem(afterFirst!, "scrap_metal_bulk"), - CountBagItem(afterReplay!, "scrap_metal_bulk")); - Assert.Equal( - salvageAfterFirst, - deps.SkillProgressionStore.GetXpTotals(PlayerId).GetValueOrDefault("salvage")); + Assert.True(deps.InstanceStore.TryGet(PlayerId, instanceId, out var instance)); + Assert.Equal(ContractInstanceStatus.Completed, instance.Status); + Assert.False(deps.InstanceStore.TryGetActiveForPlayer(PlayerId, out _)); Assert.True( deps.DeliveryStore.TryGet( PlayerId, RewardDeliverySourceKinds.ContractCompletion, instanceId, out _)); + Assert.Single(deps.OutcomeStore.GetOutcomesForInstance(instanceId)); + } + + private static string IssueAndCompleteOnce(CompletionTestDependencies deps) + { + var issued = IssuePrototypeContract(deps); + var instanceId = issued.Snapshot!.ContractInstanceId; + var first = CompleteOnEncounterClear(deps, EncounterId); + Assert.True(first.Success); + Assert.Null(first.ReasonCode); + Assert.Equal(ContractInstanceStatus.Completed, first.Snapshot!.Status); + return instanceId; } private static ContractIssueOperationResult IssuePrototypeContract(CompletionTestDependencies deps) => diff --git a/server/NeonSprawl.Server.Tests/Game/Quests/QuestFixtureApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Quests/QuestFixtureApiTests.cs index 69a7ab1..3a22039 100644 --- a/server/NeonSprawl.Server.Tests/Game/Quests/QuestFixtureApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Quests/QuestFixtureApiTests.cs @@ -3,9 +3,13 @@ using System.Net.Http.Json; using System.Text; using Microsoft.AspNetCore.Hosting; using Microsoft.Extensions.DependencyInjection; +using NeonSprawl.Server.Game.Contracts; using NeonSprawl.Server.Game.Factions; +using NeonSprawl.Server.Game.Items; +using NeonSprawl.Server.Game.Mastery; using NeonSprawl.Server.Game.Quests; using NeonSprawl.Server.Game.Rewards; +using NeonSprawl.Server.Game.Skills; using NeonSprawl.Server.Tests; using Xunit; @@ -271,4 +275,84 @@ public sealed class QuestFixtureApiTests Assert.False(deliveryStore.TryGet(DevPlayer, ChainQuestId, out _)); Assert.Empty(auditStore.GetDeltasForPlayer(DevPlayer)); } + + [Fact] + public async Task PostQuestFixture_ShouldClearContractStores_WhenResetContractInstanceIdsProvided() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + var templateRegistry = factory.Services.GetRequiredService(); + var instanceStore = factory.Services.GetRequiredService(); + var outcomeStore = factory.Services.GetRequiredService(); + var deliveryStore = factory.Services.GetRequiredService(); + var standingStore = factory.Services.GetRequiredService(); + var itemRegistry = factory.Services.GetRequiredService(); + var inventoryStore = factory.Services.GetRequiredService(); + var skillRegistry = factory.Services.GetRequiredService(); + var skillProgressionStore = factory.Services.GetRequiredService(); + var levelCurve = factory.Services.GetRequiredService(); + var perkUnlockEngine = factory.Services.GetRequiredService(); + var perkStore = factory.Services.GetRequiredService(); + var auditStore = factory.Services.GetRequiredService(); + var timeProvider = factory.Services.GetRequiredService(); + const string templateId = "prototype_contract_clear_combat_pocket"; + const string encounterId = "prototype_combat_pocket"; + var issued = ContractGeneratorOperations.TryIssue( + DevPlayer, + templateId, + "2026-06-22", + zoneDifficultyBand: null, + templateRegistry, + instanceStore, + standingStore, + timeProvider); + Assert.True(issued.Success); + var instanceId = issued.Snapshot!.ContractInstanceId; + var completed = ContractCompletionOperations.TryCompleteOnEncounterClear( + DevPlayer, + encounterId, + instanceStore, + outcomeStore, + templateRegistry, + itemRegistry, + inventoryStore, + skillRegistry, + skillProgressionStore, + levelCurve, + perkUnlockEngine, + perkStore, + deliveryStore, + standingStore, + auditStore, + timeProvider); + Assert.True(completed.Success); + Assert.True( + deliveryStore.TryGet( + DevPlayer, + RewardDeliverySourceKinds.ContractCompletion, + instanceId, + out _)); + Assert.Single(outcomeStore.GetOutcomesForInstance(instanceId)); + + // Act + var reset = await client.PostAsJsonAsync( + FixturePath, + new QuestFixtureRequest + { + SchemaVersion = QuestFixtureRequest.CurrentSchemaVersion, + ResetContractInstanceIds = [instanceId], + }); + + // Assert + Assert.Equal(HttpStatusCode.OK, reset.StatusCode); + Assert.False(instanceStore.TryGet(DevPlayer, instanceId, out _)); + Assert.False( + deliveryStore.TryGet( + DevPlayer, + RewardDeliverySourceKinds.ContractCompletion, + instanceId, + out _)); + Assert.Empty(outcomeStore.GetOutcomesForInstance(instanceId)); + } } diff --git a/server/NeonSprawl.Server/Game/Contracts/ContractCompletionOperations.cs b/server/NeonSprawl.Server/Game/Contracts/ContractCompletionOperations.cs index c18909a..6d150e5 100644 --- a/server/NeonSprawl.Server/Game/Contracts/ContractCompletionOperations.cs +++ b/server/NeonSprawl.Server/Game/Contracts/ContractCompletionOperations.cs @@ -119,7 +119,20 @@ public static class ContractCompletionOperations return Success(afterAttempt, TryGetExistingOutcome(outcomeStore, afterAttempt)); } - return Deny(ContractCompletionReasonCodes.InvalidIds, activeSnapshot); + if (!instanceStore.CanWritePlayer(normalizedPlayerId)) + { + return Deny(ContractCompletionReasonCodes.PlayerNotWritable, activeSnapshot); + } + + if (instanceStore.TryGet( + normalizedPlayerId, + activeSnapshot.ContractInstanceId, + out var currentAfterFailedMark)) + { + return Deny(ContractInstanceReasonCodes.InstanceNotActive, currentAfterFailedMark); + } + + return Deny(ContractInstanceReasonCodes.InstanceNotFound, activeSnapshot); } private static ContractOutcomeRow? AppendOutcome( From 6f735f8643d2b7eef6263ba22ceaf6c0c33fcfbb Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 27 Jun 2026 21:52:13 -0400 Subject: [PATCH 5/7] NEO-149: fix analyzer warnings in encounter wiring RCS1123 parentheses in quest wiring test, CA1873 guarded LogDebug, CA1859 List parameter for CompensatingRemoveGrants. --- .../Game/Quests/QuestObjectiveWiringTests.cs | 2 +- .../Game/Encounters/EncounterCombatWiring.cs | 15 ++++++++++----- .../Encounters/EncounterCompletionOperations.cs | 2 +- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/server/NeonSprawl.Server.Tests/Game/Quests/QuestObjectiveWiringTests.cs b/server/NeonSprawl.Server.Tests/Game/Quests/QuestObjectiveWiringTests.cs index 1debd92..28d1b73 100644 --- a/server/NeonSprawl.Server.Tests/Game/Quests/QuestObjectiveWiringTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Quests/QuestObjectiveWiringTests.cs @@ -45,7 +45,7 @@ public sealed class QuestObjectiveWiringTests Assert.True(deps.ProgressStore.TryGetProgress(PlayerId, GatherQuestId, out var progress)); Assert.Equal(QuestProgressStatus.Completed, progress.Status); Assert.Equal( - 3 * GatherSkillXpConstants.ActivityXpPerResourceNodeGather + 25, + (3 * GatherSkillXpConstants.ActivityXpPerResourceNodeGather) + 25, deps.XpStore.GetXpTotals(PlayerId).GetValueOrDefault("salvage")); Assert.True(deps.DeliveryStore.TryGet(PlayerId, GatherQuestId, out var deliveryEvent)); Assert.Single(deliveryEvent.GrantedSkillXp); diff --git a/server/NeonSprawl.Server/Game/Encounters/EncounterCombatWiring.cs b/server/NeonSprawl.Server/Game/Encounters/EncounterCombatWiring.cs index 2f08862..7f4c3df 100644 --- a/server/NeonSprawl.Server/Game/Encounters/EncounterCombatWiring.cs +++ b/server/NeonSprawl.Server/Game/Encounters/EncounterCombatWiring.cs @@ -1,3 +1,4 @@ +using Microsoft.Extensions.Logging; using NeonSprawl.Server.Game.Contracts; using NeonSprawl.Server.Game.Factions; using NeonSprawl.Server.Game.Items; @@ -115,11 +116,15 @@ public static class EncounterCombatWiring if (!completion.Success) { - logger?.LogDebug( - "Encounter completion grant skipped for player {PlayerId} encounter {EncounterId} ({ReasonCode})", - playerId, - encounterId, - completion.ReasonCode); + if (logger?.IsEnabled(LogLevel.Debug) == true) + { + logger.LogDebug( + "Encounter completion grant skipped for player {PlayerId} encounter {EncounterId} ({ReasonCode})", + playerId, + encounterId, + completion.ReasonCode); + } + return; } diff --git a/server/NeonSprawl.Server/Game/Encounters/EncounterCompletionOperations.cs b/server/NeonSprawl.Server/Game/Encounters/EncounterCompletionOperations.cs index 17cfa2e..9b7142c 100644 --- a/server/NeonSprawl.Server/Game/Encounters/EncounterCompletionOperations.cs +++ b/server/NeonSprawl.Server/Game/Encounters/EncounterCompletionOperations.cs @@ -212,7 +212,7 @@ public static class EncounterCompletionOperations private static void CompensatingRemoveGrants( string playerId, - IReadOnlyList grants, + List grants, IItemDefinitionRegistry itemRegistry, IPlayerInventoryStore inventoryStore) { From eb6a4feddebd94b011d0634b5a7ae65d34285b5e Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 27 Jun 2026 21:56:52 -0400 Subject: [PATCH 6/7] NEO-149: fix CraftResult and craft API analyzer warnings Move CraftResult param docs to primary-constructor summaries (RCS1263), use CraftIoAppliedJson[] return and collection expression in MapIoRows, prefer is true for debug log guard (CDT1001), and add MapResponse deny test. --- .../Post craft gather refine make spine.bru | 2 +- .../Game/Crafting/PlayerCraftApiTests.cs | 22 +++++++++++++++++++ .../Game/Crafting/CraftResult.cs | 6 ++--- .../Game/Crafting/PlayerCraftApi.cs | 4 ++-- .../Game/Encounters/EncounterCombatWiring.cs | 2 +- 5 files changed, 29 insertions(+), 7 deletions(-) diff --git a/bruno/neon-sprawl-server/craft/Post craft gather refine make spine.bru b/bruno/neon-sprawl-server/craft/Post craft gather refine make spine.bru index 4fe7cf6..c42d955 100644 --- a/bruno/neon-sprawl-server/craft/Post craft gather refine make spine.bru +++ b/bruno/neon-sprawl-server/craft/Post craft gather refine make spine.bru @@ -5,7 +5,7 @@ meta { } docs { - NEO-70 prototype spine: accumulate >=11 scrap_metal_bulk (gather when nodes allow, reuse persisted inventory after dotnet test), refine_scrap_efficient when refined_plate_stock < 2, then make_field_stim_mk0. NEO-118: best-effort quest objective wiring on craft success (active craft_recipe quests). No inventory POST shortcuts for spine materials. + NEO-70 prototype spine: accumulate >=11 scrap_metal_bulk (gather when nodes allow, reuse persisted inventory after dotnet test), refine_scrap_efficient when refined_plate_stock < 2, then make_field_stim_mk0. NEO-118: best-effort quest objective wiring on craft success (active craft_recipe quests). Deny envelopes return empty inputsConsumed/outputsGranted arrays. No inventory POST shortcuts for spine materials. } script:pre-request { diff --git a/server/NeonSprawl.Server.Tests/Game/Crafting/PlayerCraftApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Crafting/PlayerCraftApiTests.cs index 5838719..3488291 100644 --- a/server/NeonSprawl.Server.Tests/Game/Crafting/PlayerCraftApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Crafting/PlayerCraftApiTests.cs @@ -266,6 +266,28 @@ public sealed class PlayerCraftApiTests Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); } + [Fact] + public void MapResponse_ShouldReturnEmptyIoArrays_WhenCraftDenied() + { + // Arrange + var result = new CraftResult( + Success: false, + ReasonCode: CraftReasonCodes.UnknownRecipe, + InputsConsumed: [], + OutputsGranted: [], + XpGrantSummary: null); + + // Act + var response = PlayerCraftApi.MapResponse(result); + + // Assert + Assert.False(response.Success); + Assert.Equal(CraftReasonCodes.UnknownRecipe, response.ReasonCode); + Assert.Empty(response.InputsConsumed); + Assert.Empty(response.OutputsGranted); + Assert.Null(response.XpGrantSummary); + } + private static void SeedStack(InMemoryWebApplicationFactory factory, string itemId, int quantity) { var itemRegistry = factory.Services.GetRequiredService(); diff --git a/server/NeonSprawl.Server/Game/Crafting/CraftResult.cs b/server/NeonSprawl.Server/Game/Crafting/CraftResult.cs index fe69884..720756c 100644 --- a/server/NeonSprawl.Server/Game/Crafting/CraftResult.cs +++ b/server/NeonSprawl.Server/Game/Crafting/CraftResult.cs @@ -4,12 +4,12 @@ namespace NeonSprawl.Server.Game.Crafting; /// Server-internal craft resolution envelope (NEO-69); promoted to wire JSON via (NEO-70). /// NEO-71 telemetry hook sites live in (success path + centralized Deny helper). /// -/// Scaled inputs removed on success; empty when denied. -/// Scaled outputs added on success; empty when denied. -/// Refine skill XP applied on success; null when denied. public readonly record struct CraftResult( bool Success, string? ReasonCode, + /// Scaled inputs removed on success; empty when denied. IReadOnlyList InputsConsumed, + /// Scaled outputs added on success; empty when denied. IReadOnlyList OutputsGranted, + /// Refine skill XP applied on success; null when denied. CraftXpGrantSummary? XpGrantSummary); diff --git a/server/NeonSprawl.Server/Game/Crafting/PlayerCraftApi.cs b/server/NeonSprawl.Server/Game/Crafting/PlayerCraftApi.cs index a7ce75a..009e9b5 100644 --- a/server/NeonSprawl.Server/Game/Crafting/PlayerCraftApi.cs +++ b/server/NeonSprawl.Server/Game/Crafting/PlayerCraftApi.cs @@ -96,11 +96,11 @@ public static class PlayerCraftApi }; } - private static IReadOnlyList MapIoRows(IReadOnlyList rows) + private static CraftIoAppliedJson[] MapIoRows(IReadOnlyList rows) { if (rows.Count == 0) { - return Array.Empty(); + return []; } var mapped = new CraftIoAppliedJson[rows.Count]; diff --git a/server/NeonSprawl.Server/Game/Encounters/EncounterCombatWiring.cs b/server/NeonSprawl.Server/Game/Encounters/EncounterCombatWiring.cs index 7f4c3df..b173c07 100644 --- a/server/NeonSprawl.Server/Game/Encounters/EncounterCombatWiring.cs +++ b/server/NeonSprawl.Server/Game/Encounters/EncounterCombatWiring.cs @@ -116,7 +116,7 @@ public static class EncounterCombatWiring if (!completion.Success) { - if (logger?.IsEnabled(LogLevel.Debug) == true) + if (logger?.IsEnabled(LogLevel.Debug) is true) { logger.LogDebug( "Encounter completion grant skipped for player {PlayerId} encounter {EncounterId} ({ReasonCode})", From c5b8b7a7268906ae7d9d36602588a1907aeaeab7 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 27 Jun 2026 21:58:26 -0400 Subject: [PATCH 7/7] NEO-149: re-review after code review follow-ups --- docs/reviews/2026-06-27-NEO-149.md | 34 ++++++++++++++++++------------ 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/docs/reviews/2026-06-27-NEO-149.md b/docs/reviews/2026-06-27-NEO-149.md index 06fd154..d143329 100644 --- a/docs/reviews/2026-06-27-NEO-149.md +++ b/docs/reviews/2026-06-27-NEO-149.md @@ -1,25 +1,30 @@ # Code review — NEO-149 contract completion encounter wiring **Date:** 2026-06-27 -**Scope:** Branch `NEO-149-e7m4-06-contract-completion-encounter-wiring` — commits `8d9d02f`..`bcf6fd2` vs `origin/main` -**Base:** `origin/main` +**Scope:** Branch `NEO-149-e7m4-06-contract-completion-encounter-wiring` — commits `8d9d02f`..`eb6a4fe` vs `origin/main` +**Base:** `origin/main` +**Follow-up:** Re-review after code-review follow-ups (`68ba267`–`eb6a4fe`). ## Verdict -**Approve with nits** +**Approve** ## Summary -NEO-149 adds `ContractCompletionOperations.TryCompleteOnEncounterClear` — deliver-then-mark contract completion on encounter clear, following the NEO-128 quest precedent and NEO-148 router. The orchestrator resolves the active instance, matches `encounterTemplateId`, calls `TryDeliverContractCompletion`, marks the instance complete, and appends an outcome audit row. Encounter wiring invokes the hook after encounter loot and quest objective wiring; contract payout failure does not block encounter success (result discarded). Contract store/registry dependencies are plumbed through `EncounterCombatWiring` and `AbilityCastApi`. `QuestFixtureRequest.resetContractInstanceIds` clears instance, delivery, and outcome rows for Bruno/dev reset loops. Five unit tests, one DI integration test, and one encounter-completion wiring test cover match/mismatch, delivery deny, idempotent replay, and combined encounter + contract loot. Implementation plan reconciliation, `server/README.md`, and E7.M4 module anchor are updated. Server-only slice; client work explicitly deferred to NEO-153/NEO-154 after NEO-151 HTTP. Full suite: **904** tests passed locally. +NEO-149 adds `ContractCompletionOperations.TryCompleteOnEncounterClear` — deliver-then-mark contract completion on encounter clear, following the NEO-128 quest precedent and NEO-148 router. Encounter wiring invokes the hook after encounter loot and quest objective wiring; contract payout failure does not block encounter success. Contract store/registry dependencies are plumbed through `EncounterCombatWiring` and `AbilityCastApi`. `QuestFixtureRequest.resetContractInstanceIds` clears instance, delivery, and outcome rows for Bruno/dev reset loops. + +**Follow-up commits** address all prior review suggestions: E7.M4 alignment register updated, `PostQuestFixture_ShouldClearContractStores_WhenResetContractInstanceIdsProvided` added, `TryMarkComplete` failure path returns `player_not_writable` / `instance_not_active` / `instance_not_found` instead of `invalid_ids`, and idempotent replay test refactored with `IssueAndCompleteOnce`. Ancillary analyzer fixes in encounter wiring (debug log guard) and craft API (`CraftResult` XML docs, `MapIoRows` return type, deny empty IO arrays test) keep the branch clean; craft changes are orthogonal to contract completion but low risk. + +Seven contract-related tests (5 unit + 1 integration + 1 encounter wiring) plus 1 quest-fixture API test cover the story. Full suite: **906** tests passed locally. ## Documentation checked | Path | Result | |------|--------| -| `docs/plans/NEO-149-implementation-plan.md` | **Matches** — AC checklist complete; shipped reconciliation aligns with diff | +| `docs/plans/NEO-149-implementation-plan.md` | **Matches** — AC checklist complete; shipped reconciliation aligns with diff (test count still cites 904 — minor drift) | | `docs/plans/E7M4-pre-production-backlog.md` (E7M4-06) | **Matches** — orchestrator + encounter wiring in scope; HTTP Bruno loop deferred to NEO-151 | | `docs/decomposition/modules/E7_M4_ContractMissionGenerator.md` | **Matches** — NEO-149 completion wiring anchor added | -| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E7.M4 row includes **E7M4-06 / NEO-149** (orchestrator + encounter wiring + quest-fixture reset) | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E7.M4 row includes **E7M4-06 / NEO-149** | | `server/README.md` | **Matches** — contract completion operations section, wiring order, reason codes, fixture reset | | Client counterpart (NEO-153, NEO-154) | **N/A** — explicitly out of scope; no prototype-complete claim | @@ -31,24 +36,25 @@ None. ## Suggestions -1. ~~**Update implementation alignment register** — Add **E7M4-06 / NEO-149 landed** to the E7.M4 row in `docs/decomposition/modules/documentation_and_implementation_alignment.md` (orchestrator + encounter wiring + quest-fixture reset), consistent with NEO-144–NEO-148 entries.~~ **Done.** E7.M4 row + plan link updated. +1. ~~**Update implementation alignment register** — Add **E7M4-06 / NEO-149 landed** to the E7.M4 row in `docs/decomposition/modules/documentation_and_implementation_alignment.md` (orchestrator + encounter wiring + quest-fixture reset), consistent with NEO-144–NEO-148 entries.~~ **Done.** E7.M4 row + plan link updated (`68ba267`). -2. ~~**Quest-fixture test for contract reset** — `QuestFixtureOperations` now clears contract instance, delivery, and outcome rows via `resetContractInstanceIds`, but `QuestFixtureApiTests` has no parallel to `PostQuestFixture_ShouldClearRewardDeliveryAndAudit_WhenResetQuestIdsProvided`. A test that issues a contract, completes it, resets via fixture, and asserts stores are empty would lock the NEO-148 follow-on reset path.~~ **Done.** `PostQuestFixture_ShouldClearContractStores_WhenResetContractInstanceIdsProvided`. +2. ~~**Quest-fixture test for contract reset** — `QuestFixtureOperations` now clears contract instance, delivery, and outcome rows via `resetContractInstanceIds`, but `QuestFixtureApiTests` has no parallel to `PostQuestFixture_ShouldClearRewardDeliveryAndAudit_WhenResetQuestIdsProvided`. A test that issues a contract, completes it, resets via fixture, and asserts stores are empty would lock the NEO-148 follow-on reset path.~~ **Done.** `PostQuestFixture_ShouldClearContractStores_WhenResetContractInstanceIdsProvided` (`68ba267`). -3. ~~**`TryMarkComplete` failure reason code** — When delivery succeeds but `TryMarkComplete` fails without leaving a completed snapshot, the orchestrator returns `invalid_ids` (line 122 in `ContractCompletionOperations.cs`). `QuestStateOperations.TryMarkComplete` uses richer fallback codes (`NotActive`, `UnknownPlayer`). Consider aligning deny semantics in a follow-on if store-failure paths become testable; not blocking for prototype (same deliver-then-mark stuck-state risk as quest path when mark loses after delivery).~~ **Done.** Final deny uses `player_not_writable`, `instance_not_active`, or `instance_not_found` instead of `invalid_ids`. +3. ~~**`TryMarkComplete` failure reason code** — When delivery succeeds but `TryMarkComplete` fails without leaving a completed snapshot, the orchestrator returns `invalid_ids`. `QuestStateOperations.TryMarkComplete` uses richer fallback codes. Consider aligning deny semantics.~~ **Done.** Final deny uses `player_not_writable`, `instance_not_active`, or `instance_not_found` (`68ba267`). ## Nits -- Nit: Idempotent replay after successful completion returns `no_active_contract` (instance no longer active) rather than hitting the defensive `completed` snapshot branch at lines 59–61. Behavior is correct — no double grant — but the reason code differs from the plan’s “completed replay” wording. -- ~~Nit: `TryCompleteOnEncounterClear_ShouldBeIdempotent_WhenReplayed` performs success assertions on the first completion inside **Arrange**; prefer moving first-call verification to **Assert** after **Act** for strict AAA, or extract a shared setup helper.~~ **Done.** `IssueAndCompleteOnce` setup helper; replay test Act/Assert only. -- Nit: Plan lists optional `EncounterCombatWiring` end-to-end issue → cast → contract completed; only compile-fix call-site updates landed. Acceptable given integration + encounter completion tests cover the path. -- Nit: Bruno `Reset contract instance via quest fixture.bru` uses placeholder `ci_000…` id — fine until NEO-151 wires issue → clear → GET; helper `resetContractInstances` in `bruno-dev-fixture-helper.js` is the useful piece for that story. +- Nit: Idempotent replay after successful completion returns `no_active_contract` (instance no longer active) rather than hitting the defensive `completed` snapshot branch. Behavior is correct — no double grant — but the reason code differs from the plan’s “completed replay” wording. **Deferred** — acceptable for prototype. +- ~~Nit: `TryCompleteOnEncounterClear_ShouldBeIdempotent_WhenReplayed` performs success assertions on the first completion inside **Arrange**; prefer moving first-call verification to **Assert** after **Act** for strict AAA, or extract a shared setup helper.~~ **Done.** `IssueAndCompleteOnce` setup helper (`68ba267`). +- Nit: Plan lists optional `EncounterCombatWiring` end-to-end issue → cast → contract completed; only compile-fix call-site updates landed. **Deferred** — integration + encounter completion tests cover the path. +- Nit: Bruno `Reset contract instance via quest fixture.bru` uses placeholder `ci_000…` id — fine until NEO-151 wires issue → clear → GET. +- Nit: Follow-up commits include craft API analyzer/doc fixes unrelated to E7M4-06 scope; harmless but could have been a separate chore commit. ## Verification ```bash dotnet test NeonSprawl.sln -dotnet test NeonSprawl.sln --filter "FullyQualifiedName~ContractCompletionOperations|FullyQualifiedName~EncounterCompletionOperationsTests.TryCompleteAndGrant_ShouldApplyContractBundle" +dotnet test NeonSprawl.sln --filter "FullyQualifiedName~ContractCompletionOperations|FullyQualifiedName~QuestFixtureApiTests.PostQuestFixture_ShouldClearContractStores" ``` Manual Godot QA: none (server engine per plan). Bruno issue → clear → GET loop: NEO-151.