path: root/FIXES.md

# Fix Tranche 1 — verified P0/P1 plus high-value P2s

Date: 2026-10-05
Companion to `AUDIT.md` (findings) and `reports/audit/` (raw lane reports).

## Status

- `go build ./...` — pass
- `go vet ./...` — pass
- `make lint` (`golangci-lint run`) — **now 0 issues** (was 15)
- `go test ./...` — pass
- `go test -race` (engine, net, persistence, player, world, combat, casino, game, hacking) — pass
- Changes are uncommitted. `git diff --shortstat` = 51 files, +509/−131 (plus new test files).

Nothing was staged or committed. The audit artifacts (`AUDIT.md`, `reports/`) are locally excluded via `.git/info/exclude` so they do not dirty the tree.

## Fixed (each with a regression test unless noted)

### Correctness / exploit (P0/P1)
1. **Gather verbs broken (P0)** — restored `mine/chop/fish/cut → gather` aliases in `internal/game/act.go`; corrected the misleading comment. Test: `TestGatherVerbsNormalizeToGatherAction`.
2. **`walk <huge>n` remote OOM** — `parseWalkDirections` now clamps the count before allocating (`internal/game/cmd_walk.go`). Test: `TestParseWalkDirectionsRejectsHugeCount`.
3. **`sell`/`deposit` dead-end on 2+ stacks** — dedup by name with `uniqueItemNames` (`cmd_shop.go`, `cmd_bank.go`).
4. **Admin `//` path bypass** — added `canonicalDataRelPath`, classify on the canonical path, removed the shadowed `fullPath` join (`internal/admin/api_files.go`); hardened the game-side `playerDataFileName` the same way. Tests: `TestCanonicalDataRelPathStripsRepeatedPrefixes`, `TestPlayerDataFileNameCanonicalizesPrefixes`.
5. **Undo can truncate a live file to 0 bytes** — `load()` maps an empty before-image back to nil so `requireSnapshot` fires (`internal/admin/undo.go`). (Fail-safe: an intentionally empty before-image is refused rather than written; the explicit-existence-flag fix remains a follow-up.)
6. **Roulette selective-refund exploit** — disconnect during `phasePlaying` now forfeits the stake, matching Big 6 (`internal/casino/roulette.go`); updated `TestRouletteRefundAndPersistenceFailure` to assert forfeit vs refund.
7. **Resolved predictions destroyed by the credit cap** — compute totals and run the cap guard before mutating contract status (`internal/game/predictions.go`). Test: `TestCollectPredictionsKeepsContractsWhenCreditCapBlocks`.
8. **`RekeyRooms` stranded wanderers** — rebuild the object-state map from every state and remap `RoomID`/`HomeRoomID`/`WanderRooms` (`internal/world/world.go`). Test: `TestRekeyRoomsUpdatesAwayFromHomeObject`.
9. **Admin room swap/undo did not reconcile live state** — `handleRoomSwap` now reconciles both rooms; `rebuildAfterUndo` reconciles when the room file still exists (`internal/admin/api_rooms.go`, `server.go`).
10. **Estate directory was dead** — allow `estate_directory` through the interactability gate (`act.go`); exempt the `Cfg == nil && EstateDirectory` prompt in `advanceTalk` (`act_talk.go`). Live content places it in `data/rooms/tuccia/3124.yaml`.
11. **`ui.Table` title overhang** — truncate the title to the interior width (`internal/ui/table.go`). Test: `TestTableLongTitleStaysWithinBorder`.
12. **Hacking minigames printed raw color tags** — expand tags before writing (`internal/game/core_hacking.go`).
13. **Pai gow betting window never expired with one bettor + one idle seat** — count seated players, not bettors (`internal/casino/pai_gow.go`). Test: `TestPaiGowBettingWindowExpiresWithOneBettorAndIdleSeat`.

### Combat / player / items
14. **Double combat entry** — `startCombat` now refuses re-entry (`internal/game/combat_attack.go`); `Tracker.Enter` frees the previous mob (`internal/combat/state.go`). Test: `TestTrackerEnterReleasesPreviousMob`.
15. **Retribution skipped `StartRegen`** — surviving mob no longer heals 100× too fast (`internal/game/combat_mob.go`).
16. **`RemoveItem` drained items on failure** — now all-or-nothing (`internal/player/player.go`). Test: `TestRemoveItemIsAllOrNothing`.
17. **`drop all <item>` zeroed quality** — preserve Quality/MaxQuality via `AddGroundItemQ` (`internal/game/cmd_drop.go`).
18. **`RegenerateTick` was persisted** — added `yaml:"-"` (`internal/player/player.go`).
19. **`XPTable` was `[100]int` with 99 values** — now `[99]int` (`internal/player/xp.go`). Test: `TestXPTableBounds`.

### Command / UX
20. **Prefix/abbreviation lowercased arguments** — preserve original case for `rawInput` consumers (`internal/game/game.go`). Test: `TestGlobalCommandPreservesArgumentCase`.
21. **`queued` ignored the consume slot** — added `PeekConsume` and display (`core_command_queue.go`, `cmd_queued.go`).
22. **`queued` hardcoded 600 ms** — now reports the configured tick length (`cmd_queued.go`).
23. **`verbs` ignored `wrap_width`** — honors the option (`cmd_verbs.go`).
24. **Unknown craft subtype soft-locked the action** — fallback uses `behavior.TypeCraft` so it advances (`production_engine.go`).
25. **Empty-ingredient craft was infinite/free** — `planIngredients` returns nil (`production_index.go`). Test: `TestPlanIngredientsRejectsEmptyCraft`.

### Persistence / validation / net
26. **`Move` + `Delete`/`Replace` silently lost files** — conflicting ops now error (`internal/persistence/txn.go`). Tests: `TestTxnRejectsConflictingMoveThenDelete`, `TestTxnRejectsConflictingMoveThenReplace`, `TestTxnMoveThenReplaceDestination`.
27. **`Recover` dropped unknown op kinds and deleted the journal** — unknown kind is now a rollback error, keeping the journal (`txn.go`). Test: `TestRecoverKeepsJournalWithUnknownOpKind`.
28. **Journal directory entry not durable** — `Recover` (the startup path that creates it) now fsyncs its parent (`txn.go`).
29. **`ExitDef.Condition` never validated** — added the check (`internal/validate/checks.go`).
30. **`stripIAC` leaked a byte for a truncated IAC** — drop the remainder (`internal/net/conn.go`). Test: `TestStripIACDropsTruncatedSequence`.

### Tooling
31. **`make lint` gate restored** — fixed the 15 reported issues (error-string casing, doc-comment form, unused params, ineffassign) without weakening `.golangci.yml`.

## Deliberately deferred (still open)

These are documented in `AUDIT.md`; the next tranche should cover them.

- **Undo failure reporting** (`Undo()`/`Redo()` still return nil on failure → "Nothing to undo"). Needs a signature change.
- **Explicit snapshot-existence flag** to allow undoing an intentionally empty file (current fix is a safe refusal).
- **`quit` queue/busy interaction**; **bank withdraw duplicate ambiguity list**; **`get <n> credits` ignores `<n>`**.
- **Production `FirstCraft()` vs multi-recipe availability**; **"how many" batches vs units**.
- **Room object reference entries silently drop behavior fields**; **`.yml` files invisible to validation**; **craft `type`/`subtype` validator rule**; **talk `start` node check**; **safespot validation**; **drop weight validation**.
- **Persistence `Recover` mid-game misuse**, journal versioning, `.save-*` temp sweep.
- **Casino**: craps Fire/ATS deferral, shooter-change reset, video-poker raw error text, slots free-spin off-by-one, plinko RTP wording, machine leave silently dropping a banked feature, cabinet ID collisions.
- **Admin**: `restoreAccountSecret` fail-open, `entityCreate` persisting `_path`/`_raw`, trigger POST/PUT divergence, `validEntityID`, `resolve-duplicate`, course detach, symlink handling.
- **Net**: telnet echo CRLF deadline, shutdown flush starvation, `armReadDeadline` concrete-type switch, hub bucket pruning.
- **Structural refactors** from `AUDIT.md` §3–4 (Table shrink consolidation, `countExtraInbound`, cert gen, YAML encoders, `tableCore` for pai gow, typed `GridConflict.Kind`, etc.).
- **Test infrastructure**: `make test-race`/CI, harness owner-goroutine leak, master-tick harness option, and coverage for steal/gather/burn/talk and the WebSocket transport.

## Suggested next step

Continue with a second tranche focused on the remaining P1/P2 correctness items and the admin/persistence/validation gaps, then a structural-refactor tranche. Each change should keep `make lint`, `go test ./...`, and `go test -race` green.