aboutsummaryrefslogtreecommitdiff
path: root/FIXES.md
diff options
context:
space:
mode:
authorhistoria <[not public]>2026-10-06 12:57:23 -0400
committerhistoria <[not public]>2026-10-06 12:57:23 -0400
commit9666676766bf21364f4fc4d0d1d336e72f2a91b4 (patch)
tree0cade34091d7602790dc38cd44053f0b74bbadf2 /FIXES.md
parent4bc1daaf259e0c19c5fe4a12ec8bc7f6e5a735c1 (diff)
downloadthehouseoficarus-9666676766bf21364f4fc4d0d1d336e72f2a91b4.tar.gz
refactor: split casino.go, dedup helpers, remove audit docs
Diffstat (limited to 'FIXES.md')
-rw-r--r--FIXES.md136
1 files changed, 0 insertions, 136 deletions
diff --git a/FIXES.md b/FIXES.md
deleted file mode 100644
index f89cce9..0000000
--- a/FIXES.md
+++ /dev/null
@@ -1,136 +0,0 @@
-# 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`.
-
-## Tranche 2
-
-- **Security**: `restoreAccountSecret` no longer fails open on a non-mapping body (the admin raw editor can no longer blank `password_hash`); `validEntityID` rejects control characters and leading dots.
-- **Info disclosure**: video-poker bank/wager errors are mapped instead of printing raw `os`/YAML errors and server paths.
-- **Casino**: slots free-spin counter no longer prints "0 free spins remain" before the last spin; `leave` settles banked free-spin winnings instead of silently dropping them; craps repeated-odds returns the correct error; a craps shooter change resets the hand (point/made/seen) so the next shooter gets a fresh come-out.
-- **Economy**: `get <n> credits` honors the requested quantity; bank withdraw ambiguity list no longer duplicates entries.
-- **Admin**: `entityCreate` drops editor-only `_path`/`_raw`; `detachRoomFromCourse` checks `os.Remove` and detaches from every matching course.
-- **Validation**: `.yml` files are discovered/validated (matching the runtime trigger loader); talk configs with nodes but no `start` node are an error.
-- **Persistence**: `Recover` sweeps crash-created `.save-*` temp files.
-- **Net**: `Conn` now requires `SetReadDeadline`, so every transport gets an idle read deadline (removed the concrete-type switch); hub room buckets are pruned and `RekeyRooms` merges instead of clobbering.
-- Tests added: `TestRestoreAccountSecretRefusesNonMappingBody`, `TestWalkYAMLDirIncludesYml`, `TestValidateTalkConfigRequiresStartNode`, `TestPickupCreditsHonorsRequestedQuantity`, `TestMachineLeavePaysBankedFeature`, `TestCrapsShooterChangeResetsHand`.
-
-## Tranche 3 (this pass)
-
-- **Undo**: a failed undo/redo is now reported distinctly from an empty history (`UndoStack.LastError()`), so the admin UI no longer shows "Nothing to undo" for a real failure. Test: `TestUndoFailureIsReportedDistinctly`.
-- **Quit**: `quit`/`logout` are `ClassInstant`, so a queued active command can no longer silently replace or delay a quit.
-- **World**: a reference-style room object entry (`id:` only) that carries behavior fields is rejected at load instead of silently dropping the behavior. Test: `TestRoomObjectReferenceRejectsBehaviorFields`.
-- **Validation**: gather drops with `weight <= 0` (never selectable) are warned about. Test: `TestValidateGatherWarnsOnWeightlessDrop`.
-- **Admin**: trigger POST now serializes through `triggerMapToYAML` like PUT (no unknown-key divergence); `resolve-duplicate` verifies the path is genuinely a duplicate-ID room file before deleting/renaming it.
-- **Net**: the shutdown flush gives each session a bounded slice instead of letting the first stalled writer starve the rest; the telnet echo CRLF refreshes its write deadline so it is not dropped on an idle connection.
-- **Casino**: video-poker cabinet IDs are claimed in the addressable namespace, so an ID colliding with a machine variant/game name is rejected instead of producing an unreachable cabinet. Test: `TestVideoPokerCabinetIDCollisionIsRejected`.
-
-## Tranche 4 (this pass)
-
-- **Production**: the multi-recipe bug is fixed. Availability scanned every recipe but execution used `Craft[0]`, so 5 of 6 `arrow_shaft` recipes were unreachable (start-then-abort). A `CraftIndex` is now carried through `ProductionData`/`CombineData`/`FletchData`; `usableCraftIndex` selects the recipe the player satisfies; `collectByproducts`/`canContinueProduction` use the selected recipe; the production menu displays the usable recipe; and `findFletchItems` scans every recipe. Test: `TestUsableCraftIndexSelectsSatisfiedRecipe`.
-- **Validation**: mob loot and steal drops with `weight <= 0` are now warned about (gather already was).
-- **Persistence**: journals now carry a schema `version`; `Recover` refuses a version it does not understand instead of misreading a newer journal. Test: `TestRecoverRefusesUnknownJournalVersion`.
-- **Casino**: the plinko rules text now states the measured per-risk returns rather than only the target RTP.
-- **Refactor**: the duplicated `countExtraInbound` (game + admin, byte-for-byte) is now one `world.CountExtraInbound`, so the two room-removal paths cannot drift.
-
-## Tranche 5 (this pass)
-
-- **Refactor**: the three YAML map encoders (`admin.marshalYAML`, `admin.marshalMapYAML`, `behavior.MarshalYAMLMap`) now share one `behavior.MarshalYAML`.
-- **Refactor**: the `.journal` path is now `persistence.JournalDir(dataDir)` used by both the game and the admin API, so a future rename cannot leave one writer journaling to a directory startup recovery never scans.
-- **Test infrastructure**: added a `make test-race` target (`go test -race ./... -timeout 300s`); the shared game harness now stops the Game owner goroutine on cleanup (previously every harness test leaked one); added the first WebSocket transport tests (`TestWebSocketCheckOrigin` unit-tests the origin boundary with a non-loopback peer, `TestWebSocketUpgradeAccepted` drives the `/ws` upgrade end-to-end).
-
-## Tranche 6 (this pass)
-
-- **Refactor**: `GridConflict.Kind` is now the typed `world.GridConflictKind` (`GridConflictTwist`/`GridConflictOverlap`), replacing the magic strings switched on in five packages.
-- **Refactor**: the duplicated self-signed cert generator is now `tlscert.SelfSigned(commonName)`, shared by the game and admin servers.
-- **Validation**: craft `type`/`subtype` is checked against `item.KnownCraftKeys` — an unknown key left the action set but never advancing (player stuck). Test: `TestValidateCraftBlocksRejectsUnknownType`.
-- **Validation**: the mob/safespot size vocabulary moved to `object.MobSizeRank`; `validateObjects` rejects an unknown `safespot.max_block_size` (silently disables cover) and out-of-range chances, and `validateMobCombatStats` rejects an unknown mob `size`. Test: `TestValidateObjectsRejectsUnknownSafespotSize`.
-
-## Tranche 7 (this pass)
-
-- **Security**: `resolveDataPath` now resolves symlinks on the longest existing prefix and re-checks containment, so a symlink inside `data/` cannot redirect a raw-editor read or write outside the data directory. Test: `TestResolveDataPathRejectsSymlinkEscape`.
-- **UX**: the production "how many" prompt now says "How many batches (each makes N, return for all)?" so a batch count cannot silently mean N× the materials/units.
-- **Test coverage**: added `TestGatherYieldsDrop`, an end-to-end gather test (seed object + pickaxe, start the action, tick to a drop) — the first coverage for the gather system the audit flagged.
-- **Investigated (no change)**: the craps Fire bet. Settling it at the 4th/5th point as the audit suggested would cap the payout at the lower tier and under-pay; the bet must ride to the seven-out to pay the highest tier reached. Left as-is and noted here.
-
-## Tranche 8 (this pass)
-
-- **Undo**: added an explicit before/after snapshot-existence flag (`ChangeDesc.OldExists`/`NewExists`, carried through `ExtraFile` and the persisted `undoEntry`). `requireSnapshot` now keys on existence, not `len == 0`, so an intentionally empty file can be undone after a restart while a missing (failed-read) image is still refused. Older history files infer the flag from non-empty content. Test: `TestUndoRestoresIntentionallyEmptyFile`.
-- **Refactor**: the duplicated `world.remapInt` and `game.remapScalar` are now one exported `world.RemapInt`, used by both the room-file and player-state migrations.
-
-## Tranche 9 (this pass)
-
-- **Test coverage**: added `TestBurnCreatesFire`, an end-to-end firemaking test (seed logs + tinderbox, start the action, tick until a fire object appears) — the burn coverage the audit called out.
-- **Test infrastructure**: added `newHarnessWithTicks`, a harness variant that registers the production master tick subscription, plus `TestHarnessWithTicksCompletesDisconnect`, which exercises the real `Hub.Remove → DisconnectTick → onRemove` countdown end-to-end (the gap the audit found in the disconnect race tests).
-- **Refactor**: exported `ui.FitWidths` (the `Table.Render` shrink algorithm) and replaced the duplicated shrink loop in `casino_house_edge.go`, so the two cannot disagree on which column yields.
-
-## Tranche 10 (this pass)
-
-- **Bug fix (previously missed)**: a terminal talk node (messages, no options, no goto) now clears the talk action in `showTalkNode`, so it no longer lingers as a stale action that suppresses the prompt and no-ops every tick. Test: `TestTerminalTalkNodeClearsAction`.
-- **Test coverage**: added `TestObjectStealYieldsLoot`, an end-to-end object-steal test (seed a stealable stall, start the action, tick to loot). Talk and steal are now covered (gather and burn were in earlier tranches).
-- **Refactor**: `paiGowTable.phase` is now the typed `tablePhase` with `phaseSetting`/`phaseSettling` constants, removing the 29 raw phase-string literals (a safe subset of the `tableCore` consistency work).
-- **Docs**: `recoverJournals` now documents that it must run only at startup or on the owner goroutine, making the mid-game `Recover` invariant explicit.
-
-## Still open
-
-- **Casino**: craps Fire bet forfeiture on disconnect (design decision; shutdown refunds).
-- **Structural refactors** remaining from `AUDIT.md` §3–4 (`tableCore` for pai gow — now partly addressed by the typed phase — `refs` struct in validate, flex-aware predictions table).
-- **CI**: wire `make test-race` into a workflow.
-
-## Suggested next step
-
-The remaining structural refactors are cosmetic DRY with real regression risk and no functional benefit; the correctness backlog is now empty. Suggest stopping here unless a specific item is wanted. Keep `make lint`, `go test ./...`, and `go test -race` green.