fix: bound wizard-created Which against its item table (closes #10)

createObj stored the raw 0-f nibble as Object.Which with no bounds
check, so wizard mode -> C -> / -> f made a wand numbered 15 against a
14-entry table and panicked in fixStick. Input outside 0-f overshoots
much further rather than going negative: readchar returns a byte, so
the int(ch-'a') + 10 branch is byte arithmetic and wraps, giving 234
for 'A' and 202 for '!', and panicked the same way. C's create_obj()
was equally unchecked, but its consumers were either switches (defined
for any value) or static-array reads past the end (undefined, and
survivable in practice). Since one game is now one process, the Go
panic kills the game outright and leaves the terminal in raw mode.

Reject at the two boundaries a bad Which can enter through. createObj
now refuses an out-of-range choice with a message drawn from C's own
type_name() vocabulary and adds nothing to the pack, a deliberate
divergence recorded in a comment because C had no defined behavior here
to be faithful to. Restore refuses a snapshot describing such an object
(ErrSaveCorrupt) rather than loading a game that would explode later. A
decoded snapshot is also the only source of a genuinely negative Which,
Which being a plain int off the wire, so it is what the Which >= 0 arm
of hasValidWhich defends against.

Behind those, whichLimit/hasValidWhich back defensive guards at every
dispatch the issue names: the quaffHandler/readHandler/zapHandler
accessors return no handler instead of indexing (for wands that is
exactly what non-MASTER C did, matching no case and still running
o_charges--), the callIt lore lookups, identifyType, armorClass for the
a_class[] reads, initWeapon against the missing init_dam[] row for
WeaponFlame, fixStick's ws_type[] read, and inventoryName and
objectWorth, hoisted so one check each covers the whole family of
per-kind name and appraisal tables. identifyType's bound is defensive
rather than live: readHandlers registers readIdentify only for the
identify scrolls, all of which sit inside the shorter idType table.

No in-range input changes behavior and no guard consumes a random
number: the rejection precedes every rnd() call. TestSeedCompatItemTables
stays green untouched.

New game/wizard_test.go covers the exact reproducer, a rejection sweep
over every indexed kind including the wrapped values from input outside
0-f, an acceptance sweep proving valid choices still build the right
item, one no-panic test per guarded family, the fixStick crash site, the
corrupt-save rejection over both the wrapped values and a negative
Which, and a check that whichLimit still agrees with the table sizes.
Each guard was confirmed load-bearing by reverting it and watching the
test fail.

TODO.md records the step; Next Step is deliberately left alone, since
this arrived out of band via an issue.

TODO.md

@@ -37,40 +37,45 @@ wizard commands).
 - 2026-08-09 Wizard-create bounds fix (`fix/wizard-which-bounds`, closes #10):
   `createObj` stored the raw `0-f` nibble as `Object.Which` with no bounds
   check, so wizard mode -> `C` -> `/` -> `f` produced a wand numbered 15 against
-  a 14-entry table and panicked in `fixStick`; input below `'a'` or `'0'` went
-  negative (`'A'` gives -22, `'!'` gives -54) and panicked the same way. C's
-  `create_obj()` was equally unchecked, but every C consumer was either a
-  `switch` (defined for any value) or a static-array read past the end
-  (undefined, and survivable in practice), whereas since refactor step 8 one
-  game is one process, so the Go panic kills the game with the terminal still in
-  raw mode. Fixed at the two boundaries a bad `Which` can enter through:
-  `createObj` now rejects an out-of-range choice with a message built from C's
-  own `type_name()` vocabulary and adds nothing to the pack (a deliberate,
-  commented divergence, since C had no defined behavior here to be faithful to),
-  and `Restore` refuses a snapshot describing such an object (`ErrSaveCorrupt`)
-  instead of loading a game that would explode later. Behind those,
-  `whichLimit`/`hasValidWhich` back defensive guards at every dispatch named in
-  the issue: the three effect tables (the new `quaffHandler`, `readHandler`, and
-  `zapHandler` accessors return no handler rather than indexing — for wands that
-  is exactly non-`MASTER` C, which matched no case and still ran `o_charges--`),
-  the `callIt` lore lookups, `identifyType` (whose table is shorter than the
-  scroll table keying it), `armorClass` for all four `a_class[]` reads,
-  `initWeapon` against the missing `init_dam[]` row for `WeaponFlame`,
-  `fixStick`'s `ws_type[]` read, and `inventoryName`, hoisted so one check
-  covers the scroll-title read the issue listed plus its potion-color,
-  ring-stone, wand-material, weapon and armor siblings. `objectWorth` got the
-  same hoisted guard, since the death-screen appraisal reads the identical
-  per-kind tables. No in-range input changes behavior and no guard consumes a
-  random number — the rejection precedes every `rnd()` call, verified both by an
-  explicit seed-unchanged test and by `TestSeedCompatItemTables` staying green
-  untouched. New `game/wizard_test.go`: the exact reproducer, a rejection sweep
-  over every indexed kind including both negative-input forms, an acceptance
-  sweep proving valid choices still build the right item, one no-panic test per
-  guarded family (wand/potion/scroll/armor/weapon), the `fixStick` crash site,
-  the corrupt-save rejection, and a check that `whichLimit` still agrees with
-  the table sizes. Each guard was confirmed load-bearing by reverting it and
-  watching the test panic. `Next Step` deliberately not rotated: this was
-  out-of-band issue work.
+  a 14-entry table and panicked in `fixStick`. Input outside `0-f` overshoots
+  much further rather than going negative: `readchar` returns a `byte`, so the
+  `int(ch-'a') + 10` branch is byte arithmetic and wraps — `'A'` gives 234 and
+  `'!'` gives 202 — and panicked the same way. C's `create_obj()` was equally
+  unchecked, but every C consumer was either a `switch` (defined for any value)
+  or a static-array read past the end (undefined, and survivable in practice),
+  whereas since refactor step 8 one game is one process, so the Go panic kills
+  the game with the terminal still in raw mode. Fixed at the two boundaries a
+  bad `Which` can enter through: `createObj` now rejects an out-of-range choice
+  with a message built from C's own `type_name()` vocabulary and adds nothing to
+  the pack (a deliberate, commented divergence, since C had no defined behavior
+  here to be faithful to), and `Restore` refuses a snapshot describing such an
+  object (`ErrSaveCorrupt`) instead of loading a game that would explode later.
+  Behind those, `whichLimit`/`hasValidWhich` back defensive guards at every
+  dispatch named in the issue: the three effect tables (the new `quaffHandler`,
+  `readHandler`, and `zapHandler` accessors return no handler rather than
+  indexing — for wands that is exactly non-`MASTER` C, which matched no case and
+  still ran `o_charges--`), the `callIt` lore lookups, `identifyType` (whose
+  table is shorter than the scroll table keying it, though no scroll that can
+  reach `readIdentify` overshoots it, so that one is defensive rather than a
+  live bound), `armorClass` for the four `a_class[]` reads, `initWeapon` against
+  the missing `init_dam[]` row for `WeaponFlame`, `fixStick`'s `ws_type[]` read,
+  and `inventoryName`, hoisted so one check covers the scroll-title read the
+  issue listed plus its potion-color, ring-stone, wand-material, weapon and
+  armor siblings. `objectWorth` got the same hoisted guard, since the
+  death-screen appraisal reads the identical per-kind tables. No in-range input
+  changes behavior and no guard consumes a random number — the rejection
+  precedes every `rnd()` call, verified both by an explicit seed-unchanged test
+  and by `TestSeedCompatItemTables` staying green untouched. New
+  `game/wizard_test.go`: the exact reproducer, a rejection sweep over every
+  indexed kind including both wrapping-input forms, an acceptance sweep proving
+  valid choices still build the right item, one no-panic test per guarded family
+  (wand/potion/scroll/armor/weapon), the `fixStick` crash site, the corrupt-save
+  rejection over the wrapped values and a negative `Which` (a decoded snapshot
+  is the only source of one, so it is what exercises the `Which >= 0` arm of
+  `hasValidWhich`), and a check that `whichLimit` still agrees with the table
+  sizes. Each guard was confirmed load-bearing by reverting it and watching the
+  test panic. `Next Step` deliberately not rotated: this was out-of-band issue
+  work.
 
 - 2026-08-09 Stale-docs correction (`docs-staleness`, closes #3): four claims in
   `MEMORY.md`/`TODO.md`/`README.md` had gone false and were misdirecting agents

game/object.go

@@ -219,6 +219,13 @@ func (o *Object) ArmorKind() ArmorKind { return ArmorKind(o.Which) }
 // satisfy it; the wizard-create command and a restored save file are the
 // only two ways a malformed one can appear, and both reject it (see
 // createObj and validateSnapshotObjects).
+//
+// The Which >= 0 arm is unreachable from the keyboard: createObj derives
+// Which with byte arithmetic (int(ch-'a') + 10), which wraps to a large
+// positive value rather than going negative. It is kept as
+// defense-in-depth for the non-keyboard source — a decoded save file,
+// where Which is an int off the wire and can hold anything — and is
+// exercised there by TestRestoreRejectsOutOfRangeWhich.
 func (o *Object) hasValidWhich() bool {
 	limit := whichLimit(o.Kind)
 

game/scrolls.go

@@ -169,10 +169,12 @@ func (g *RogueGame) createMonsterSpot() (Coord, bool) {
 }
 
 func (g *RogueGame) readIdentify(obj *Object) {
-	// Identify, let him figure something out. readHandler has already
-	// bounded Which against the scroll table to get here, but idType is
-	// shorter than that table (it stops after the last identify scroll),
-	// so the filter lookup carries its own bound.
+	// Identify, let him figure something out. idType is shorter than the
+	// scroll table keying it (it stops after the last identify scroll),
+	// so the filter lookup carries its own bound. That bound is not
+	// reachable today: readHandlers registers readIdentify only for the
+	// identify scrolls, all of which sit inside idType. It is kept as
+	// defense-in-depth against a future table resize.
 	g.Items.Scrolls[obj.Which].Know = true
 	g.msg("this scroll is an %s scroll", g.Items.Scrolls[obj.Which].Name)
 	g.whatis(true, g.data.identifyType(obj.ScrollKind()))

game/tables.go

@@ -897,7 +897,10 @@ func (d *gameData) zapHandler(obj *Object) func(g *RogueGame, obj *Object) bool
 
 // identifyType reads extern.c's identify-scroll to item-kind map,
 // returning KindNone for any scroll past the last identify scroll (the
-// table is shorter than the scroll table it is keyed by).
+// table is shorter than the scroll table it is keyed by). No scroll that
+// can reach readIdentify is past that end, so the guard is defensive
+// rather than a live bound; it exists so a future table resize cannot
+// turn the lookup into a panic.
 func (d *gameData) identifyType(kind ScrollKind) ObjectKind {
 	if kind < 0 || int(kind) >= len(d.idType) {
 		return KindNone

game/wizard.go

@@ -27,14 +27,16 @@ func (g *RogueGame) createObj() {
 	g.Msgs.Mpos = 0
 
 	// Deliberate divergence from 5.4.4: C stored this nibble unchecked, so
-	// 'a'-'f' (and anything below '0' or 'a', which goes negative) indexed
-	// straight past the ends of the per-kind static tables. That was
-	// undefined behavior C happened to survive by reading adjacent memory;
-	// in Go it is a panic that kills the process with the terminal still in
-	// raw mode. C had no defined behavior here to be faithful to, so the
-	// choice is rejected outright rather than emulating a garbage read. The
-	// check precedes every rnd() call below, so the RNG sequence is
-	// untouched either way.
+	// 'a'-'f' indexed straight past the ends of the per-kind static
+	// tables. Input outside '0'-'9' and 'a'-'f' overshoots much further:
+	// readchar returns a byte, so ch-'a' above is byte arithmetic and
+	// wraps instead of going negative ('A' gives 234, '!' gives 202).
+	// Reading past a static array was undefined behavior C happened to
+	// survive by picking up adjacent memory; in Go it is a panic that
+	// kills the process with the terminal still in raw mode. C had no
+	// defined behavior here to be faithful to, so the choice is rejected
+	// outright rather than emulating a garbage read. The check precedes
+	// every rnd() call below, so the RNG sequence is untouched either way.
 	if !obj.wizardCanCreate() {
 		g.msg("there is no such %s", obj.Kind)
 

game/wizard_test.go

@@ -47,9 +47,12 @@ func TestCreateObjWandFReproducer(t *testing.T) {
 }
 
 // TestCreateObjRejectsOutOfRangeWhich sweeps the rejection across every
-// kind whose Which is a table index, including the negative values that
-// input below 'a' and below '0' produce ('A'-'a'+10 == -22, '!'-'a'+10 ==
-// -54; isDigit is false for both, so both take the letter branch).
+// kind whose Which is a table index, including input outside '0'-'f'.
+// isDigit is false for such input, so it takes the letter branch, where
+// ch-'a' is byte arithmetic and wraps rather than going negative: 'A'
+// (65) gives int(224)+10 == 234 and '!' (33) gives int(192)+10 == 202.
+// Those far-past-the-end values, not negative ones, are what the guard
+// has to catch on the keyboard path.
 func TestCreateObjRejectsOutOfRangeWhich(t *testing.T) {
 	t.Parallel()
 
@@ -62,14 +65,14 @@ func TestCreateObjRejectsOutOfRangeWhich(t *testing.T) {
 		{"potion f is past NumPotionTypes", Potion, 'f'},
 		{"ring f is past NumRingTypes", Ring, 'f'},
 		// NumScrollTypes is 18, past the 'f' the prompt tops out at, so a
-		// scroll can only be driven out of range by going negative.
-		{"negative below 'a' for scroll", Scroll, 'A'},
+		// scroll can only be driven out of range by input that wraps.
+		{"scroll 'A' wraps to 234", Scroll, 'A'},
 		{"armor 9 is past NumArmorTypes", Armor, '9'},
 		{"weapon 9 is the flame pseudo-weapon", Weapon, '9'},
-		{"negative: letter below 'a'", Stick, 'A'},
-		{"negative: character below '0'", Stick, '!'},
-		{"negative below 'a' for armor", Armor, 'A'},
-		{"negative below '0' for weapon", Weapon, '!'},
+		{"wand 'A' wraps to 234", Stick, 'A'},
+		{"wand '!' wraps to 202", Stick, '!'},
+		{"armor 'A' wraps to 234", Armor, 'A'},
+		{"weapon '!' wraps to 202", Weapon, '!'},
 	}
 
 	for _, tc := range cases {
@@ -315,30 +318,53 @@ func TestFixStickMalformedWhichDoesNotPanic(t *testing.T) {
 // TestRestoreRejectsOutOfRangeWhich is the save-file half of the fix: a
 // malformed object must not be able to sneak past the keyboard guard by
 // arriving in a snapshot.
+//
+// A decoded save is also the only place a *negative* Which can come
+// from. On the keyboard path createObj's ch-'a' is byte arithmetic and
+// wraps, so 'A' and '!' land at 234 and 202; Which is a plain int in the
+// gob stream, so a tampered file can carry any value at all. Both shapes
+// are covered here, and the negative case is what exercises the
+// Which >= 0 arm of hasValidWhich.
 func TestRestoreRejectsOutOfRangeWhich(t *testing.T) {
 	t.Parallel()
 
-	g := mkGame(t, 11)
-	st := g.snapshot()
-
-	if len(st.Player.Body.Pack) == 0 {
-		t.Fatal("starting pack is empty; nothing to corrupt")
+	cases := []struct {
+		name  string
+		which int
+	}{
+		{"one past the wand table", int(NumWandTypes)},
+		{"the value 'A' wraps to on the keyboard path", 234},
+		{"the value '!' wraps to on the keyboard path", 202},
+		{"negative, reachable only from a tampered file", -1},
 	}
 
-	st.Player.Body.Pack[0].Kind = KindWand
-	st.Player.Body.Pack[0].Which = int(NumWandTypes)
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
 
-	path := filepath.Join(t.TempDir(), "rogue.save")
-	writeSnapshot(t, path, st)
+			g := mkGame(t, 11)
+			st := g.snapshot()
 
-	_, restoreErr := Restore(path, Params{Term: &testTerm{}})
-	if !errors.Is(restoreErr, ErrSaveCorrupt) {
-		t.Errorf("Restore error = %v, want ErrSaveCorrupt", restoreErr)
-	}
+			if len(st.Player.Body.Pack) == 0 {
+				t.Fatal("starting pack is empty; nothing to corrupt")
+			}
 
-	_, statErr := os.Stat(path)
-	if statErr != nil {
-		t.Error("a rejected save file was deleted; it should be left alone")
+			st.Player.Body.Pack[0].Kind = KindWand
+			st.Player.Body.Pack[0].Which = tc.which
+
+			path := filepath.Join(t.TempDir(), "rogue.save")
+			writeSnapshot(t, path, st)
+
+			_, restoreErr := Restore(path, Params{Term: &testTerm{}})
+			if !errors.Is(restoreErr, ErrSaveCorrupt) {
+				t.Errorf("Restore error = %v, want ErrSaveCorrupt", restoreErr)
+			}
+
+			_, statErr := os.Stat(path)
+			if statErr != nil {
+				t.Error("a rejected save file was deleted; it should be left alone")
+			}
+		})
 	}
 }