Created phase 10 to fix bugs

This commit is contained in:
bepis
2026-10-22 18:42:07 +11:00
parent 3086e24836
commit d115f49eaf
2 changed files with 72 additions and 1 deletions
+68 -1
View File
@@ -7,7 +7,7 @@
> - Keep Session Log entries to one or two lines.
## Current Focus
- **Phase:** Complete (Phase 9 done)
- **Phase:** 10.1z
- **Step:** Stretch Goals
- **Blocked?** No
- **Last verified working command:** `npm test` — 176/176 passing (85 unit + 91 integration + 1 e2e), 2026-10-21 step 9.6
@@ -125,6 +125,73 @@
- [ ] S6 In-game chat via polling (no websockets).
- [ ] S7 Achievements.
## Phase 10 — Repair & Completion (docs: GAME_DESIGN, BALANCE, API, SECURITY, TESTING)
**Rules for every Phase 10 step (read before starting):**
1. **Reproduce first.** Write the failing test BEFORE the fix. The test must fail on the old code.
2. **Assert values, not just status codes.** Read the DB or response and `assert.equal` exact numbers (energy, xp, gold, row counts). No `if (...)` guards that can skip an assertion. No `assert.ok([200, 400].includes(status))`.
3. **Never delete or weaken an existing test.** The passing-test count must only go up.
4. **One implementation per rule.** If two files compute the same thing, extract ONE shared function and use it in both. Don't add a third copy.
5. **Check the frontend.** If you change an API response shape, update the `public/js/*.js` that reads it.
6. **Use `utils/time.js` for time and an injectable `rng` for randomness** (never `Date.now()` / `Math.random()` in services).
7. **Balance changes need a NEW migration.** Never edit an applied migration.
---
### A. Expedition bugs (verified live)
- [ ] **10.1** Fix expedition START corrupting energy. In `startExpedition` (`src/services/expeditionService.js`) the transaction function's parameters are in a different order than the call, so the DB gets `energy = def.energy_cost` and `energy_updated_at = endsAt`. Fix so the DB stores `energy - cost` and the timestamp returned by `computeEnergy`.
*Acceptance:* test: hero energy 100, start the 10-energy expedition → DB `energy` is 90 and `energy_updated_at` is NOT in the future; `GET /api/character` shows 90; response energy equals DB energy. Test also starting at energy 55 → 45.
- [ ] **10.2** Fix expedition CLAIM overwriting XP. Claim must ADD the reward to current XP, run `checkLevelUp`, ADD (not overwrite) unspent points, and heal to the new max HP on level-up. Extract ONE shared function (e.g. `applyXpGain` in `src/game/leveling.js` or a small progression service) and use it from BOTH `huntService` and `expeditionService`.
*Acceptance:* tests: xp 80 + reward 30 at level 1 → level 2, xp 10, unspent_points +3; xp 0 + 30 → xp 30, level 1; a reward spanning two levels → both levels gained, points +6; double claim still rejected; gold added correctly. Existing hunt tests still pass.
- [ ] **10.3** Make expeditions testable: use `time.now()` and an injectable `rng` (no `Date.now()` / `Math.random()` in `expeditionService.js`).
*Acceptance:* test with a fixed rng asserts the exact gold reward; a test using `time.setNow` proves claim is rejected before `ends_at` and accepted after.
### B. Accounts & navigation
- [ ] **10.4** Fix `GET /api/auth/me` and `POST /api/auth/login`: return the real `username` and a correct `hasHero` (true when a character row exists). Store the username in the session at login/register. Update `public/js/login.js` and `public/js/create-hero.js`: returning players with a hero go to `/game.html`; players without go to `/create-hero.html`; `/create-hero.html` redirects to `/game.html` if a hero already exists.
*Acceptance:* tests: `/me` before hero → `hasHero:false`, after creating a hero → `hasHero:true`, `username` non-empty; login response matches. No frontend code assumes `hasHero` is always false.
- [ ] **10.5** Add a landing page `public/index.html` (title, one-line tagline, Login/Register buttons; "Play" button if already logged in with a hero). Fix the "Return to Emberfall" links on the 404/500 pages so they work. Add a link-checker test that reads every `public/*.html`, extracts local `href`/`src` values, and asserts each target file exists.
*Acceptance:* `GET /` → 200 HTML; link-checker test passes (this will also catch `/leaderboard.html` until 10.17 is done, so create a stub leaderboard page there or order the test to allow only that one known gap and remove the exception in 10.17).
### C. Security & robustness
- [ ] **10.6** Restore REAL rate limiting. In `src/routes/auth.js` the login and register limiters are no-ops (the real ones are commented out). Re-enable login 10 / 15 min / IP and register 5 / hour / IP, using the standard error envelope (`ok:false`, code `RATE_LIMITED`, status 429). Mount the global `/api` limiter BEFORE the routers (it is currently registered after the auth routes, so it doesn't cover them). Limits may be relaxed only when `NODE_ENV=test`, via config, not by stubbing.
*Acceptance:* a test that enables the real limiter: 10 bad logins → 401, the 11th → 429 JSON. Remove the "rate limiters mounted" tick in STATUS 9.1 notes if it was wrong, and record the correction in the Session Log.
- [ ] **10.7** Error handling gaps: malformed JSON → 400 `INVALID_JSON`; body over the limit → 413 `PAYLOAD_TOO_LARGE`; unknown `/api/*` path → 404 JSON `{ok:false,...}`; all as JSON, never an HTML page. Replace `throw` inside callbacks (e.g. `req.session.regenerate((err) => { throw ... })`) with `next(err)` so a store error can't crash the process.
*Acceptance:* tests for each of the three responses (exact status + `error.code`); a test that forces `regenerate` to fail and asserts a 500 JSON response and that the server stays up.
### D. Progression: stat points, HP, equipment
- [ ] **10.8** Add `POST /api/character/allocate` (API.md): body `{str,agi,int,vit}` as points to add (zod `.strict()`, non-negative integers). Use the existing, tested `allocatePoints` from `src/game/leveling.js`. One transaction; reject if the total exceeds `unspent_points` (422 `NOT_ENOUGH_POINTS`); return `{character, derived}`.
*Acceptance:* tests: spend 2 of 3 points on STR → `str` +2, `unspent_points` 1, derived attack increased by the exact expected amount; spending 4 of 3 → 422 and nothing changed; negative / float / unknown keys → 400; another user's character cannot be affected.
- [ ] **10.9** Allocation UI: on the dashboard stats panel show unspent points and a `+` button per stat (only when points remain), with a confirm/apply button, loading state, and error toast. Use `textContent`, never `innerHTML` with dynamic data.
*Acceptance:* in the frontend, applying points calls the endpoint and refreshes stats without a page reload. Add a link-checker/DOM-free test only if feasible; otherwise list manual verification steps in HANDOFF.md.
- [ ] **10.10** Add `POST /api/character/rest` (API.md, BALANCE.md §Rest): costs `level * 5` gold, heals to full max HP. Errors: `NOTHING_TO_HEAL` (422) if HP is already full, `INSUFFICIENT_GOLD` (422). One transaction. Add a "Rest (N gold)" button to the dashboard.
*Acceptance:* tests: HP 10/135 level 3 with gold 100 → HP 135, gold 85; full HP → 422 and gold unchanged; gold 5 → 422; HP unchanged on failure.
- [ ] **10.11** Natural HP regeneration (GAME_DESIGN §Time): +1% of max HP per minute, computed lazily like energy. New migration (next unused number) adds `hp_updated_at INTEGER` (backfill existing rows with `energy_updated_at`). Add a pure function `computeHp(current, maxHp, updatedAtMs, nowMs)` in `src/game/hp.js` with unit tests (partial minute, cap at max, clock backwards, large gap). Apply it before the HP check in hunting, in `getCharacterProfile`, in potion use, and in rest. Heals/damage must set `hp_updated_at = now`.
*Acceptance:* unit tests for `computeHp`; integration test: hero at 1 HP, 30 minutes later hunting is allowed again (≥ 20% of max HP restored by lazy regen, using `time.setNow`). A defeated hero can no longer soft-lock.
- [ ] **10.12** Equipment must matter in combat. Create ONE `getEquipmentBonuses(characterId)` function (sum of `bonus_str/agi/int/vit/attack/defense` over equipped items) and use it in `huntService` (pass the totals to `resolveCombat` as `bonus_*`), in `getCharacterProfile` (derived stats), in potion max-HP capping, and in the shop/inventory stat previews. Delete the copy-pasted aggregation code in `shopService.js` and `inventoryService.js`.
*Acceptance:* deterministic tests with a constant rng: the first hero hit is exactly higher with an equipped weapon than without; `GET /api/character` `derived.attack` includes the weapon bonus; equipment `bonus_vit` raises max HP exactly once; grep shows only one aggregation implementation.
- [ ] **10.13** Starter kit (GAME_DESIGN §Items): `createCharacter` grants, in the same transaction, one class-appropriate common weapon (warrior Rusty Sword, mage Apprentice Staff, ranger Hunter's Shortbow) already equipped in the weapon slot, plus 2 Minor Healing Potions.
*Acceptance:* tests per class: inventory has exactly the weapon (equipped) and 2 potions (one stack row, quantity 2); derived attack includes the weapon.
### E. Economy
- [ ] **10.14** Fix material sell prices. The code uses `defaultRate * heroLevel / itemMinLevel`; BALANCE.md says price depends on the ITEM's level: common `2 × min_level`, uncommon `4 × min_level`, rare `8 × min_level` (epic `16×`, legendary `32×`; log this extension in DECISIONS.md), minimum 1. Note BALANCE.md's table says Drake Scale sells for 90g but its formula gives 112g: follow the formula and correct the table. Use ONE function (`materialSellPrice` in `src/game/rewards.js`) for the shop list AND for selling.
*Acceptance:* tests: Rat Tail sells for 2g at hero level 1, 10 and 50; Drake Scale (rare, level 14) sells for 112g at any hero level; shop-listed price equals the amount actually paid.
- [ ] **10.15** Non-stackable items must be one inventory row per unit. In `buyItem`, buying N of a non-stackable item inserts N rows with quantity 1 (reuse the same grant helper that `huntService` uses; extract it to a shared place if needed). Stackable items keep merging.
*Acceptance:* tests: buy 3 swords → 3 rows, quantity 1 each; equip one, sell one → remaining state correct; buy 5 potions → one row, quantity 5.
### F. Public pages
- [ ] **10.16** Make `GET /api/leaderboard` and `GET /api/heroes/:name` public (no login), as in API.md, and keep returning only public fields. Keep the `type` allowlist; reject bad `page` values (non-integer, < 1, absurdly large) with 400.
*Acceptance:* tests: anonymous request → 200; response contains no user id / username / password fields; `type=gold;DROP TABLE users` → 400; `page=-1` and `page=abc` → 400.
- [ ] **10.17** Build `public/leaderboard.html` + `public/js/leaderboard.js`: tabs Level / Gold / Kills, rank, hero name (linking to `profile.html?name=...`), class, value; `textContent` only. Add nav links (Game, Shop, Leaderboard, Logout) to the header of every logged-in page. Remove any temporary link-checker exception from 10.5.
*Acceptance:* link-checker test passes with no exceptions; the leaderboard page works logged-out and logged-in.
### G. Balance
- [ ] **10.18** Balance pass via a NEW migration (never edit `003_content.sql`). First add `scripts/sim-balance.js`: a seeded simulation (fixed rng seed) that creates a hero per class, auto-allocates points to the class's primary stat, sells loot and rests/uses potions sensibly, and prints (a) win rate per zone at levels 1, 5, 10, (b) hunts to reach level 5 and level 10. Then tune monster stats and XP via the migration until all targets hold for all three classes:
- Level-1 hero vs Meadows: **70–90%** wins. Level-1 hero vs Crags: **≤ 15%** wins.
- Level-5 hero vs Forest: **50–75%** wins. Level-10 hero vs Crags: **50–75%** wins.
- Hunts from level 1 to 5: **40–70**. To level 10: **150–300**.
- No class is more than 15 percentage points above another in any zone.
*Acceptance:* `node scripts/sim-balance.js` prints a table showing every target met; paste the table into DECISIONS.md. Add a test that runs a small fixed-seed version and asserts the Meadows level-1 win rate is within 70–90%.
### H. Cleanup & trust
- [ ] **10.19** Repo hygiene. Remove the committed `:memory:` file and `tmp-debug-session.js` from git and add them (plus `tmp-*`) to `.gitignore`. Find why a file named `:memory:` was created (the config resolves `DB_PATH` into a real path) and make `:memory:` mean a true in-memory database. Remove the dangling `npm run seed` script (or create the file); update README.
*Acceptance:* `git ls-files` shows neither file; a config test shows `DB_PATH=:memory:` creates no file on disk; every `npm run` script in package.json points at an existing file.
- [ ] **10.20** Rewrite `tests/integration/e2e-playable.test.js` so it cannot pass vacuously: no `if` guards around assertions, exact status codes, and exact state checks via the DB between steps (energy, xp, gold, inventory, level). Cover: register → create hero (starter kit present) → hunt (energy/xp/gold change by exact amounts) → level-up and allocate points → equip → buy → sell → rest → expedition start (energy exact) → claim (xp added) → leaderboard → logout/login (`hasHero:true`). Keep or increase the total number of tests.
*Acceptance:* every `assert` on a status uses `assert.equal`; reverting the fix from 10.1 or 10.2 makes this test fail (verify once by temporarily reverting, then restore).
- [ ] **10.21** Bring the docs back to the truth. Update STATUS.md: Project Health, Test Summary (real counts), Known Issues (close what's fixed; add any known gaps, including that expeditions have no loot roll), Decisions. Fix API.md where the implementation deliberately differs (claim body takes `expeditionId`; sell/equip use `itemId`). Overwrite HANDOFF.md. Then run the manual checklist from NOTES.md §7 and record results in the Session Log.
*Acceptance:* STATUS.md numbers match `npm test`; Known Issues has no stale entries; HANDOFF.md reflects the end of Phase 10.
---
## Known Issues