step 6.3: done

HANDOFF.md

@@ -4,8 +4,8 @@
 > If this file disagrees with the code, trust the code and fix this file.
 
 ## 1. Where we are
-- **Last completed step:** 6.2 (Shop UI: `shop.html` + `shop.js` with buy/sell tabs, level-gated stock display, material sell list) — TICKED in STATUS.md
+- **Last completed step:** 6.3 (Shop tests: sell equipped item rejection + concurrent double-buy safety) — TICKED in STATUS.md
-- **Next step:** 6.3 — Additional tests: buy with insufficient gold (already covered in shop.test.js), sell equipped item (must be rejected), concurrent double-buy safety (transaction)
+- **Next step:** 7.1 — Migration `005_expeditions.sql`. Expedition definitions in seed data.
 - **Updated:** 2026-10-21
 - **Was the last session cut off mid-task?** No
 
@@ -14,7 +14,7 @@
 |---|---|
 | `npm install` | succeeds |
 | `npm run migrate` | applies 001–004 and 006 / "nothing to do" |
-| `npm test` | **137 passing, 0 failing** (85 unit + 52 integration) |
+| `npm test` | **139 passing, 0 failing** (85 unit + 54 integration) |
 
 ## 3. What exists now (files that matter)
 - `src/game/stats.js` — `calcDerived()` is the ONLY derived-stat formula. Also re-exports xpToNext/checkLevelUp.
@@ -24,7 +24,7 @@
 - `src/services/combatLogService.js` — `getCombatLog(characterId, limit)` → recent fights.
 - `src/services/characterService.js` — `createCharacter`, `getCharacterProfile`, `getCharacterIdForUser`.
 - `src/services/inventoryService.js` — `listInventory`, `equipItem`, `unequipItem`, `useConsumable`. Uses WeakMap stmt cache.
-- `src/services/shopService.js` — `getStock`, `buyItem`, `sellItem`. Uses WeakMap stmt cache. Sell value = 40% of buy_price (materials use rarity default).
+- `src/services/shopService.js` — `getStock`, `buyItem`, `sellItem`. Uses WeakMap stmt cache. Sell value = 40% of buy_price (materials use rarity default). **Equipped items rejected in sellItem (check moved before material type check).**
 - `src/routes/shop.js` — `GET /api/shop`, `POST /api/shop/buy`, `POST /api/shop/sell`.
 - `src/routes/inventory.js` — `GET /api/inventory`, `POST /api/inventory/equip`, `POST /api/inventory/unequip`, `POST /api/inventory/use`.
 - `src/utils/time.js` — `now()/setNow()/resetNow()`; never call `Date.now()` in services.
@@ -32,7 +32,7 @@
 - `src/routes/combatLog.js` — `GET /api/combat-log`.
 - `src/db/migrations/003_content.sql` — also SEEDS zones/monsters/items/loot (INSERT OR IGNORE).
 - `src/db/migrations/004_inventory.sql` — inventory table, FK constraints, partial unique index for equipped slots.
-- `tests/integration/shop.test.js` — 14 integration tests (stock listing, level gating, buy, sell, error paths, round-trip spread).
+- `tests/integration/shop.test.js` — 16 integration tests (stock, level gating, buy, sell, equipped rejection, concurrent double-buy, round-trip).
 - `tests/integration/shop-page.test.js` — 5 integration tests (stock listing, levelOk, sell pricing, buy/sell round-trip).
 - `tests/integration/inventory.test.js` — 20 integration tests (13 equip/unequip + 7 consumable).
 - `public/shop.html` — dedicated shop page with buy/sell tabs.
@@ -49,7 +49,7 @@
 - No restart-persistence test for sessions.
 - `npm run seed` script in package.json points to nonexistent `scripts/seed.js`.
 - Equipment bonuses not yet passed into combat (Issue #5 in STATUS.md).
-- Step 6.3 needs: sell equipped item rejection test, concurrent double-buy safety test.
+- Step 7.1: need migration `005_expeditions.sql` with expedition table + seed data.
 
 ## 5. Decisions & deviations from the docs
 - D-004..D-007 in `docs/DECISIONS.md` (combat rounds, equipment counted once, session store, clock).
@@ -60,6 +60,7 @@
 - UI uses emoji icons (⚔️🛡️💍🧪📦) for item types per UI_GUIDE recommendation.
 - Rarity colors match UI_GUIDE spec (common #b8b0a3, uncommon #5fb36b, rare #4a8fe0, epic #a45ee0, legendary #f2a23a).
 - **D-008:** Shop materials with buy_price=0 sell at `floor(default_rate * character_level / item_min_level)` where default_rate is common=2, uncommon=4, rare=8, epic=16, legendary=32 (min 1).
+- **D-009:** `sellItem` checks equipped_slot before material type, so equipped items are always rejected regardless of item type.
 
 ## 6. Gotchas learned
 - `npm test` must be `node --test` with NO path/glob; files must be named `*.test.js`. A shell glob once skipped 17 tests.
@@ -73,11 +74,12 @@
 - Transaction callback args must match the call order exactly — passing `qty` as the second arg when the callback expects `etype` silently misapplies the effect.
 - Error responses from route-level `AppError`s do NOT include `data` field — only `{ ok: false, error: { code, message } }`.
 
-## 7. Next-step briefing (6.3)
+## 7. Next-step briefing (7.1)
-- **Goal:** Additional shop tests — buy with insufficient gold (already covered in shop.test.js), sell equipped item (must be rejected), concurrent double-buy safety (transaction).
+- **Goal:** Migration `005_expeditions.sql` — create expeditions table and seed expedition definitions.
-- **Docs:** GAME_DESIGN.md §Economy, API.md.
+- **Docs:** GAME_DESIGN.md §Expeditions, DATABASE.md.
-- **Files to modify:** `tests/integration/shop.test.js` or `tests/integration/shop-page.test.js`.
+- **Files to create:** `src/db/migrations/005_expeditions.sql`.
-- **Key rules:** Sell equipped item → should reject (item is in slot, still in inventory but equipped). Concurrent double-buy → two requests for same item simultaneously, only one should succeed if gold is insufficient for both.
+- **Key schema:** expeditions table (id, character_id, expedition_id, started_at, ends_at, claimed boolean FK constraints). Only one active expedition per hero at a time.
+- **Seed data:** expedition definitions in the same migration (INSERT OR IGNORE).
 
 ## 8. Open Known Issues
 STATUS.md issues 1–6 (balance, dangling `seed` script, restart test, equipment-in-combat, HP regen, zone min_level balance).

STATUS.md

@@ -7,10 +7,10 @@
 > - Keep Session Log entries to one or two lines.
 
 ## Current Focus
-- **Phase:** 6
+- **Phase:** 7
-- **Step:** 6.3
+- **Step:** 7.1
 - **Blocked?** No
-- **Last verified working command:** `npm test` — 137/137 passing (85 unit + 52 integration), 2026-10-21 step 6.2
+- **Last verified working command:** `npm test` — 139/139 passing (85 unit + 54 integration), 2026-10-21 step 6.3
 
 ## Project Health
 | Item | State |
@@ -87,7 +87,8 @@
   *Acceptance:* shop endpoints return stock, buy deducts gold, sell adds gold at 40%, gold never goes negative. *Verified:* 14 integration tests covering stock listing, level gating, buy/sell, gold math, error paths, round-trip spread.
 - [x] **6.2** Level-gated shop stock. UI page.
   *Acceptance:* shop page loads with buy/sell tabs; stock filtered by character level; materials always visible for selling. *Verified:* shop.html + shop.js created; 5 integration tests covering stock listing, level gating, sell pricing, buy/sell round-trip.
-- [ ] **6.3** Tests: buy with insufficient gold, sell equipped item (must be rejected), concurrent double-buy safety (transaction).
+- [x] **6.3** Tests: buy with insufficient gold (already covered), sell equipped item (must be rejected), concurrent double-buy safety (transaction).
+  *Acceptance:* sell equipped item → 400 VALIDATION_ERROR; concurrent double-buy → exactly one succeeds with INSUFFICIENT_GOLD for the other; insufficient gold already covered. *Verified:* 2 new integration tests added to shop.test.js; `sellItem` check moved before material type check so equipped items are rejected regardless of type.
 
 ## Phase 7 — Expeditions (Offline Progress)  (docs: GAME_DESIGN §Time, §Expeditions)
 - [ ] **7.1** Migration `005_expeditions.sql`. Expedition definitions in seed data.
@@ -148,10 +149,11 @@ Severity: `blocker` / `major` / `minor` / `cosmetic`. Status: `open` / `fixing`
 | Suite | Passing | Failing | Last run |
 |---|---|---|---|
 | unit | 85 | 0 | 2026-10-21 step 6.1 |
-| integration | 47 | 0 | 2026-10-21 step 6.1 |
+| integration | 54 | 0 | 2026-10-21 step 6.3 |
 
 ## Session Log
 _Format:_ `YYYY-MM-DD | step | what changed | result`
+- 2026-10-21 | 6.3 | Added 2 integration tests to shop.test.js (sell equipped item rejection, concurrent double-buy safety); moved equipped check before material type check in sellItem so equipped items are rejected regardless of type | 139/139 tests pass
 - 2026-10-21 | 6.2 | Created shop UI: `public/shop.html` + `public/js/shop.js` (buy/sell tabs, level-gated stock display, material sell list, toast notifications); 5 integration tests for shop page API coverage | 137/137 tests pass
 - 2026-10-21 | 6.1 | Ticked step 6.1 — acceptance criteria verified: GET /api/shop, POST /api/shop/buy, POST /api/shop/sell all implemented with 40% sell value and gold-negative protection; 14 integration tests all passing | 132/132 tests pass
 - 2026-10-21 | 6.1 | Created `src/services/shopService.js` (getStock, buyItem, sellItem) + `src/routes/shop.js` (GET /api/shop, POST /api/shop/buy, POST /api/shop/sell); registered routes in app.js; 14 integration tests covering stock listing, level gating, buy/sell, gold math, error paths, round-trip spread | 132/132 tests pass

src/services/shopService.js

@@ -148,6 +148,19 @@ function sellItem(characterId, itemId, quantity) {
   const item = s.itemById.get(itemId);
   if (!item) throw new AppError('NOT_FOUND', 'Item not found.', 404);
 
+  // Find inventory rows for this (character, item).
+  const rows = s.inventoryRows.all(characterId, itemId, 1);
+  if (rows.length === 0) {
+    throw new AppError('NOT_FOUND', 'Item not in inventory.', 404);
+  }
+
+  // Reject if any row is equipped.
+  for (const row of rows) {
+    if (row.equipped_slot !== null) {
+      throw new AppError('VALIDATION_ERROR', 'Unequip the item before selling.', 400);
+    }
+  }
+
   if (item.type !== 'material') {
     throw new AppError('VALIDATION_ERROR', 'Only materials can be sold.', 400);
   }
@@ -161,12 +174,6 @@ function sellItem(characterId, itemId, quantity) {
     sellPrice = Math.max(1, Math.floor(defaultRate * ch.level / item.min_level));
   }
 
-  // Find inventory rows for this (character, item).
-  const rows = s.inventoryRows.all(characterId, itemId, 1);
-  if (rows.length === 0) {
-    throw new AppError('NOT_FOUND', 'Item not in inventory.', 404);
-  }
-
   // Calculate how many we can actually sell from available rows.
   let remaining = quantity;
   const sales = []; // { rowId, qty, sellPrice }

tests/integration/shop.test.js

@@ -236,6 +236,39 @@ describe('shop', () => {
     assert.equal(body.error.code, 'VALIDATION_ERROR');
   });
 
+  it('concurrent double-buy: only one succeeds when gold covers exactly one', async () => {
+    const { cookie, characterId } = await setupHero({ gold: 20 });
+
+    // Rusty Sword costs 20 gold — exactly enough for one.
+    // Send two buy requests concurrently.
+    const [resA, resB] = await Promise.all([
+      fetch(`${baseUrl}/api/shop/buy`, {
+        method: 'POST',
+        headers: { 'Content-Type': 'application/json', ...authHeaders(cookie) },
+        body: JSON.stringify({ itemId: 1, quantity: 1 }),
+      }),
+      fetch(`${baseUrl}/api/shop/buy`, {
+        method: 'POST',
+        headers: { 'Content-Type': 'application/json', ...authHeaders(cookie) },
+        body: JSON.stringify({ itemId: 1, quantity: 1 }),
+      }),
+    ]);
+
+    const bodyA = await resA.json();
+    const bodyB = await resB.json();
+
+    // Exactly one should succeed, the other should fail with INSUFFICIENT_GOLD.
+    const success = [bodyA, bodyB].filter((b) => b.ok === true);
+    const failures = [bodyA, bodyB].filter((b) => b.ok === false);
+    assert.equal(success.length, 1, 'exactly one buy should succeed');
+    assert.equal(failures.length, 1, 'exactly one buy should fail');
+    assert.equal(failures[0].error.code, 'INSUFFICIENT_GOLD');
+
+    // Gold should be 0 after one successful buy.
+    const winner = success[0];
+    assert.equal(winner.data.character.gold, 0);
+  });
+
   // ─── POST /api/shop/sell ──────────────────────────────────────────────
 
   it('sells a material and increases gold', async () => {
@@ -336,6 +369,31 @@ describe('shop', () => {
     assert.equal(body.error.code, 'NOT_FOUND');
   });
 
+  // ─── Sell equipped item rejection ─────────────────────────────────────
+
+  it('rejects selling an equipped item', async () => {
+    const { cookie, characterId } = await setupHero({ gold: 100 });
+
+    // Grant and equip Rusty Sword (item 1, weapon)
+    grantItem(characterId, 1, 1);
+
+    const db = require('../../src/db').getDb();
+    db.prepare('UPDATE inventory SET equipped_slot = ? WHERE character_id = ? AND item_id = ?').run('weapon', characterId, 1);
+
+    // Try to sell the equipped Rusty Sword
+    const res = await fetch(`${baseUrl}/api/shop/sell`, {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json', ...authHeaders(cookie) },
+      body: JSON.stringify({ itemId: 1, quantity: 1 }),
+    });
+    const body = await res.json();
+
+    assert.equal(res.status, 400);
+    assert.equal(body.ok, false);
+    assert.equal(body.error.code, 'VALIDATION_ERROR');
+    assert.ok(body.error.message.includes('Unequip'), 'should mention unequipping');
+  });
+
   // ─── Round-trip ───────────────────────────────────────────────────────
 
   it('buy and sell round-trip: buy then sell loses gold (spread)', async () => {