fix: harden engine across casino, economy, concurrency, security, and content

5 files changed, 554 insertions, 0 deletions

diff --git a/internal/validate/checks.go b/internal/validate/checks.go
index 5eb4288..eb24721 100644
--- a/internal/validate/checks.go
+++ b/internal/validate/checks.go
@@ -13,6 +13,7 @@ import (
 	"thehouseoficarus/internal/casino"
 	"thehouseoficarus/internal/color"
 	"thehouseoficarus/internal/combat"
+	"thehouseoficarus/internal/item"
 	"thehouseoficarus/internal/world"
 )
 
@@ -351,6 +352,56 @@ func validateMobAttackTypes(id string, types []string) []Issue {
 	return issues
 }
 
+// validateMobCombatStats enforces the numbers a fighting mob needs: a positive
+// attack speed (engine.ToTicks clips 0 to 1, so a missing speed makes the mob
+// attack every tick) and a positive max hit for each declared attack type (a
+// missing/zero max hit turns that attack into a silent no-op). Task worksites
+// (kind: task) are intentionally inert and exempt — e.g.
+// data/mobs/examples/solar_panel_frame.yaml ships max_melee_hit: 0.
+func validateMobCombatStats(id string, c *world.MobCombat) []Issue {
+	var issues []Issue
+	if c.Kind == "task" {
+		return issues
+	}
+	stats := c.Stats
+	if stats.Speed <= 0 {
+		issues = append(issues, Issue{
+			Level:   "ERROR",
+			Type:    "reference",
+			Message: fmt.Sprintf("Mob %q: stats.speed must be > 0 (0 is clipped to 1 tick, so the mob would attack every tick)", id),
+		})
+	}
+	for _, t := range c.AttackTypes {
+		switch {
+		case combat.IsMeleeType(t):
+			if stats.MaxMeleeHit <= 0 {
+				issues = append(issues, Issue{
+					Level:   "ERROR",
+					Type:    "reference",
+					Message: fmt.Sprintf("Mob %q: stats.max_melee_hit must be > 0 for attack type %q", id, t),
+				})
+			}
+		case t == combat.AttackRanged:
+			if stats.MaxRangedHit <= 0 {
+				issues = append(issues, Issue{
+					Level:   "ERROR",
+					Type:    "reference",
+					Message: fmt.Sprintf("Mob %q: stats.max_ranged_hit must be > 0 for attack type %q", id, t),
+				})
+			}
+		case t == combat.AttackScience:
+			if stats.MaxScienceHit <= 0 {
+				issues = append(issues, Issue{
+					Level:   "ERROR",
+					Type:    "reference",
+					Message: fmt.Sprintf("Mob %q: stats.max_science_hit must be > 0 for attack type %q", id, t),
+				})
+			}
+		}
+	}
+	return issues
+}
+
 func validateMobs(s Source) []Issue {
 	var issues []Issue
 	itemIDs := s.Items.IDSet()
@@ -400,6 +451,10 @@ func validateMobs(s Source) []Issue {
 			issues = append(issues, validateMobAttackTypes(id, def.Combat.AttackTypes)...)
 		}
 
+		if def.Combat != nil {
+			issues = append(issues, validateMobCombatStats(id, def.Combat)...)
+		}
+
 		if def.Drops.Remains != "" && !itemIDs[def.Drops.Remains] {
 			issues = append(issues, Issue{
 				Level: "ERROR",
@@ -682,6 +737,17 @@ func validateCraftBlocks(s Source) []Issue {
 		return issues
 	}
 
+	// CraftDef.Tool names a tool *type* ("knife", "pickaxe", "fire"), not an
+	// item id: every runtime consumer matches it against ItemDef.ToolType() via
+	// hasToolType. Types such as "pickaxe" or "big_net" have no same-named item
+	// file, so membership is checked against the types the item defs define.
+	toolTypes := make(map[string]bool)
+	for _, def := range allItems {
+		if t := def.ToolType(); t != "" {
+			toolTypes[t] = true
+		}
+	}
+
 	for _, item := range allItems {
 		if len(item.Craft) == 0 {
 			continue
@@ -695,6 +761,18 @@ func validateCraftBlocks(s Source) []Issue {
 			}
 
 			for _, e := range c.Ingredients {
+				// Byproducts pair with the alternatives by index
+				// (collectByproducts reads Byproducts[j] for Items[j]), so a
+				// short or reordered list silently returns the wrong
+				// container. Require an exact pairing.
+				if len(e.Byproducts) != 0 && len(e.Byproducts) != len(e.Items) {
+					issues = append(issues, Issue{
+						Level: "ERROR",
+						Type:  "reference",
+						Message: fmt.Sprintf("Item %q (craft: %s): ingredient entry has %d byproduct(s) for %d alternative(s) — byproducts pair with items by index and must be empty or match in length",
+							item.ID, craftLabel, len(e.Byproducts), len(e.Items)),
+					})
+				}
 				for _, ingredientID := range e.Items {
 					if ingredientID != "" && !itemIDs[ingredientID] {
 						issues = append(issues, Issue{
@@ -705,6 +783,28 @@ func validateCraftBlocks(s Source) []Issue {
 						})
 					}
 				}
+				// Byproducts are delivered into inventory (or dropped) after
+				// consumption, so a typo here has no runtime error to catch it:
+				// it silently creates a phantom slot with no item definition.
+				for _, byproductID := range e.Byproducts {
+					if byproductID != "" && !itemIDs[byproductID] {
+						issues = append(issues, Issue{
+							Level: "ERROR",
+							Type:  "reference",
+							Message: fmt.Sprintf("Item %q (craft: %s): returns nonexistent byproduct %q",
+								item.ID, craftLabel, byproductID),
+						})
+					}
+				}
+			}
+
+			if c.Tool != "" && !toolTypes[c.Tool] {
+				issues = append(issues, Issue{
+					Level: "ERROR",
+					Type:  "reference",
+					Message: fmt.Sprintf("Item %q (craft: %s): requires nonexistent tool %q",
+						item.ID, craftLabel, c.Tool),
+				})
 			}
 
 			if c.Fail != "" && !itemIDs[c.Fail] {
@@ -716,6 +816,45 @@ func validateCraftBlocks(s Source) []Issue {
 				})
 			}
 
+			// A fail product or fail_message is only reachable when the recipe
+			// can actually fail. With no success formula the engine hardcodes a
+			// 100% success chance (see production_engine.go), so either field is
+			// dead data that would never show or drop. Kept as WARN (not ERROR)
+			// because a handful of shipped recipes author one without the other.
+			if !craftFailureReachable(c) {
+				if c.Fail != "" {
+					issues = append(issues, Issue{
+						Level: "WARN",
+						Type:  "semantic",
+						Message: fmt.Sprintf("Item %q (craft: %s): fail product %q is unreachable — success is always 100%% (add a success formula)",
+							item.ID, craftLabel, c.Fail),
+					})
+				}
+				if c.FailMessage != "" {
+					issues = append(issues, Issue{
+						Level: "WARN",
+						Type:  "semantic",
+						Message: fmt.Sprintf("Item %q (craft: %s): fail_message is unreachable — success is always 100%% (add a success formula)",
+							item.ID, craftLabel),
+					})
+				}
+			}
+
+			// A recipe that can fail but declares no fail: product destroys the
+			// materials it consumes: the failure branch removes the ingredients
+			// and returns nothing. Every shipped failable recipe hands back one
+			// of its own ingredients (the bars, the pottery, the cooked fish),
+			// so flag new ones rather than let the loss be silent. WARN, not
+			// ERROR: a recipe may legitimately choose to destroy materials.
+			if c.Fail == "" && (craftFailureReachable(c) || c.FailMessage != "") {
+				issues = append(issues, Issue{
+					Level: "WARN",
+					Type:  "semantic",
+					Message: fmt.Sprintf("Item %q (craft: %s): failure destroys materials with no fail product",
+						item.ID, craftLabel),
+				})
+			}
+
 			for _, sid := range c.Station {
 				if sid != "" && !objIDs[sid] {
 					issues = append(issues, Issue{
@@ -732,6 +871,18 @@ func validateCraftBlocks(s Source) []Issue {
 	return issues
 }
 
+// craftFailureReachable reports whether a recipe can ever fail. With no
+// success formula the engine treats the success chance as 1.0, so a declared
+// fail product or fail_message would never be produced or shown. A formula is
+// reachable when the chance at the required level (base, clamped to cap) is
+// below 1.0.
+func craftFailureReachable(c *item.CraftDef) bool {
+	if c.Success == nil {
+		return false
+	}
+	return behavior.SuccessChance(behavior.SuccessFormula(*c.Success), c.Level, c.Level) < 1
+}
+
 func validateDropTables(s Source) []Issue {
 	var issues []Issue
 	dropIDs := dropTableIDSet(s.DataDir)
diff --git a/internal/validate/correct_data_craft_validation_test.go b/internal/validate/correct_data_craft_validation_test.go
new file mode 100644
index 0000000..6b5e7f6
--- /dev/null
+++ b/internal/validate/correct_data_craft_validation_test.go
@@ -0,0 +1,102 @@
+package validate
+
+import (
+	"testing"
+)
+
+// Regression: CraftDef.Tool was validated against item ids, but it is a tool
+// *type* ("knife", "pickaxe", "fire"): runtime consumers match it with
+// ItemDef.ToolType() (g.hasToolType). Tool types such as pickaxe, axe, fire,
+// big_net and fly_rod have no same-named item file, so the old check rejected
+// legitimate recipes. These tests pin the tool-type namespace and the
+// byproduct/index pairing rule.
+
+func TestCraftToolTypeAcceptedWithoutSameNamedItem(t *testing.T) {
+	dir := t.TempDir()
+	// A pickaxe item defines tool.type: pickaxe but there is no item "pickaxe".
+	writeCraftItem(t, dir, "bronze_pickaxe", `id: bronze_pickaxe
+name: bronze pickaxe
+tool:
+  type: pickaxe
+  speed: 2
+`)
+	writeCraftItem(t, dir, "test_in", "id: test_in\nname: test in\n")
+	writeCraftItem(t, dir, "test_out", `id: test_out
+name: test out
+craft:
+  - type: crafting
+    tool: pickaxe
+    ingredients:
+      - items: [test_in]
+        quantity: 1
+`)
+
+	issues := validateCraftBlocks(newSource(dir))
+	for _, issue := range issues {
+		if issue.Level == "ERROR" {
+			t.Fatalf("a bare tool type must validate, got: %+v", issue)
+		}
+	}
+}
+
+func TestCraftToolTypeUnknownIsRejected(t *testing.T) {
+	dir := t.TempDir()
+	writeCraftItem(t, dir, "bronze_pickaxe", "id: bronze_pickaxe\nname: bronze pickaxe\ntool:\n  type: pickaxe\n")
+	writeCraftItem(t, dir, "test_in", "id: test_in\nname: test in\n")
+	writeCraftItem(t, dir, "test_out", `id: test_out
+name: test out
+craft:
+  - type: crafting
+    tool: made_up_type
+    ingredients:
+      - items: [test_in]
+        quantity: 1
+`)
+
+	if issues := validateCraftBlocks(newSource(dir)); !containsMsg(issues, "nonexistent tool") {
+		t.Fatalf("expected nonexistent-tool error, got: %+v", issues)
+	}
+}
+
+func TestCraftByproductLengthMismatchIsFlagged(t *testing.T) {
+	dir := t.TempDir()
+	writeCraftItem(t, dir, "test_in", "id: test_in\nname: test in\n")
+	writeCraftItem(t, dir, "test_alt", "id: test_alt\nname: test alt\n")
+	writeCraftItem(t, dir, "empty_container", "id: empty_container\nname: empty container\n")
+	// Two alternatives paired with a single byproduct: consuming the second
+	// alternative returns nothing (collectByproducts indexes by position).
+	writeCraftItem(t, dir, "test_out", `id: test_out
+name: test out
+craft:
+  - type: combine
+    ingredients:
+      - items: [test_in, test_alt]
+        quantity: 1
+        byproducts: [empty_container]
+`)
+
+	if issues := validateCraftBlocks(newSource(dir)); !containsMsg(issues, "pair with items by index") {
+		t.Fatalf("expected byproduct length-mismatch error, got: %+v", issues)
+	}
+}
+
+func TestCraftByproductLengthMatchIsAccepted(t *testing.T) {
+	dir := t.TempDir()
+	writeCraftItem(t, dir, "test_in", "id: test_in\nname: test in\n")
+	writeCraftItem(t, dir, "test_alt", "id: test_alt\nname: test alt\n")
+	writeCraftItem(t, dir, "empty_one", "id: empty_one\nname: empty one\n")
+	writeCraftItem(t, dir, "empty_two", "id: empty_two\nname: empty two\n")
+	writeCraftItem(t, dir, "test_out", `id: test_out
+name: test out
+craft:
+  - type: combine
+    ingredients:
+      - items: [test_in, test_alt]
+        quantity: 1
+        byproducts: [empty_one, empty_two]
+`)
+
+	if issues := validateCraftBlocks(newSource(dir)); len(issues) != 0 {
+		t.Fatalf("positionally paired byproducts must validate cleanly, got: %+v", issues)
+	}
+}
diff --git a/internal/validate/hardening_craft_validation_test.go b/internal/validate/hardening_craft_validation_test.go
new file mode 100644
index 0000000..5e5d83f
--- /dev/null
+++ b/internal/validate/hardening_craft_validation_test.go
@@ -0,0 +1,130 @@
+package validate
+
+import (
+	"path/filepath"
+	"testing"
+
+	"thehouseoficarus/internal/item"
+)
+
+// Regression: validateCraftBlocks must validate byproducts, the required tool,
+// and success/fail coherence. A typo in a byproduct previously passed startup
+// validation and the runtime wrote a phantom inventory slot with no item
+// definition; a tool that names no item was also unchecked; and a fail product
+// or fail_message on a recipe with no success formula can never be reached
+// (the engine hardcodes a 100% success chance).
+
+func writeCraftItem(t *testing.T, dir, name, body string) {
+	t.Helper()
+	writeFile(t, filepath.Join(dir, "items", name+".yaml"), body)
+}
+
+func TestCraftByproductMustExist(t *testing.T) {
+	dir := t.TempDir()
+	writeCraftItem(t, dir, "test_in", "id: test_in\nname: test in\n")
+	writeCraftItem(t, dir, "test_out", `id: test_out
+name: test out
+craft:
+  - type: crafting
+    ingredients:
+      - items: [test_in]
+        quantity: 1
+        byproducts: [missing_byproduct]
+`)
+	issues := validateCraftBlocks(newSource(dir))
+	if !containsMsg(issues, "nonexistent byproduct") {
+		t.Fatalf("expected nonexistent-byproduct error, got: %+v", issues)
+	}
+}
+
+func TestCraftToolMustExist(t *testing.T) {
+	dir := t.TempDir()
+	writeCraftItem(t, dir, "test_in", "id: test_in\nname: test in\n")
+	writeCraftItem(t, dir, "test_out", `id: test_out
+name: test out
+craft:
+  - type: crafting
+    tool: missing_tool
+    ingredients:
+      - items: [test_in]
+        quantity: 1
+`)
+	issues := validateCraftBlocks(newSource(dir))
+	if !containsMsg(issues, "nonexistent tool") {
+		t.Fatalf("expected nonexistent-tool error, got: %+v", issues)
+	}
+}
+
+func TestCraftFailWithoutSuccessFormulaIsWarned(t *testing.T) {
+	dir := t.TempDir()
+	writeCraftItem(t, dir, "test_in", "id: test_in\nname: test in\n")
+	writeCraftItem(t, dir, "burnt", "id: burnt\nname: burnt\n")
+	writeCraftItem(t, dir, "test_out", `id: test_out
+name: test out
+craft:
+  - type: cooking
+    fail: burnt
+    fail_message: You burn it.
+    ingredients:
+      - items: [test_in]
+        quantity: 1
+`)
+	issues := validateCraftBlocks(newSource(dir))
+	if !containsMsg(issues, "unreachable") {
+		t.Fatalf("expected unreachable fail warning, got: %+v", issues)
+	}
+	for _, issue := range issues {
+		if issue.Level == "ERROR" {
+			t.Fatalf("dead fail data must not fail startup validation: %+v", issue)
+		}
+	}
+}
+
+func TestCraftReachableFailProducesNoIssue(t *testing.T) {
+	dir := t.TempDir()
+	writeCraftItem(t, dir, "test_in", "id: test_in\nname: test in\n")
+	writeCraftItem(t, dir, "burnt", "id: burnt\nname: burnt\n")
+	writeCraftItem(t, dir, "test_out", `id: test_out
+name: test out
+craft:
+  - type: cooking
+    fail: burnt
+    fail_message: You burn it.
+    success:
+      base: 0.5
+      cap: 0.95
+      per_level: 0.02
+    ingredients:
+      - items: [test_in]
+        quantity: 1
+`)
+	if issues := validateCraftBlocks(newSource(dir)); len(issues) != 0 {
+		t.Fatalf("reachable fail branch must validate cleanly, got: %+v", issues)
+	}
+}
+
+func TestCraftFailureReachableFormula(t *testing.T) {
+	cases := []struct {
+		name   string
+		base   float64
+		perLvl float64
+		cap    float64
+		nilF   bool
+		want   bool
+	}{
+		{name: "no formula", nilF: true, want: false},
+		{name: "50% base", base: 0.5, perLvl: 0.02, cap: 0.95, want: true},
+		{name: "always succeeds", base: 1.0, perLvl: 0, cap: 1.0, want: false},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			craft := &item.CraftDef{Level: 1}
+			if !c.nilF {
+				craft.Success = &item.SuccessFormula{Base: c.base, PerLevel: c.perLvl, Cap: c.cap}
+			}
+			if got := craftFailureReachable(craft); got != c.want {
+				t.Errorf("craftFailureReachable = %v, want %v", got, c.want)
+			}
+		})
+	}
+}
diff --git a/internal/validate/hardening_mob_stats_validation_test.go b/internal/validate/hardening_mob_stats_validation_test.go
new file mode 100644
index 0000000..1ea30f9
--- /dev/null
+++ b/internal/validate/hardening_mob_stats_validation_test.go
@@ -0,0 +1,107 @@
+package validate
+
+import (
+	"path/filepath"
+	"testing"
+)
+
+// Regression: validateMobs must reject a fighting mob authored without an
+// attack speed (engine.ToTicks clips 0 to 1, so the mob would attack every
+// tick) or without a max hit for a declared attack type (a silent no-op).
+// Task worksites are explicitly exempt: data/mobs/examples/solar_panel_frame.yaml
+// ships max_melee_hit: 0 and must remain valid.
+
+func writeMob(t *testing.T, dir, name, body string) {
+	t.Helper()
+	writeFile(t, filepath.Join(dir, "mobs", name+".yaml"), body)
+}
+
+func TestMobMissingSpeedIsError(t *testing.T) {
+	dir := t.TempDir()
+	writeMob(t, dir, "test_mob", `id: test_mob
+name: test mob
+combat:
+  kind: combat
+  attack_types: [crush]
+  stats:
+    hp: 10
+    speed: 0
+    max_melee_hit: 5
+`)
+	issues := validateMobs(newSource(dir))
+	if !containsMsg(issues, "stats.speed") {
+		t.Fatalf("expected missing-speed error, got: %+v", issues)
+	}
+}
+
+func TestMobMissingMaxMeleeHitIsError(t *testing.T) {
+	dir := t.TempDir()
+	writeMob(t, dir, "test_mob", `id: test_mob
+name: test mob
+combat:
+  kind: combat
+  attack_types: [crush]
+  stats:
+    hp: 10
+    speed: 5
+    max_melee_hit: 0
+`)
+	issues := validateMobs(newSource(dir))
+	if !containsMsg(issues, "max_melee_hit") {
+		t.Fatalf("expected missing-melee-hit error, got: %+v", issues)
+	}
+}
+
+func TestMobMissingMaxRangedHitIsError(t *testing.T) {
+	dir := t.TempDir()
+	writeMob(t, dir, "test_mob", `id: test_mob
+name: test mob
+combat:
+  kind: combat
+  attack_types: [crush, ranged]
+  stats:
+    hp: 10
+    speed: 5
+    max_melee_hit: 3
+`)
+	issues := validateMobs(newSource(dir))
+	if !containsMsg(issues, "max_ranged_hit") {
+		t.Fatalf("expected missing-ranged-hit error, got: %+v", issues)
+	}
+}
+
+func TestTaskWorksiteMobExemptFromCombatStatChecks(t *testing.T) {
+	dir := t.TempDir()
+	writeMob(t, dir, "test_worksite", `id: test_worksite
+name: test worksite
+combat:
+  kind: task
+  attack_types: [crush]
+  stats:
+    hp: 10
+    speed: 0
+    max_melee_hit: 0
+`)
+	if issues := validateMobs(newSource(dir)); len(issues) != 0 {
+		t.Fatalf("task worksite must be exempt from combat-stat checks, got: %+v", issues)
+	}
+}
+
+func TestValidCombatMobProducesNoIssue(t *testing.T) {
+	dir := t.TempDir()
+	writeMob(t, dir, "test_mob", `id: test_mob
+name: test mob
+combat:
+  kind: combat
+  attack_types: [slash, ranged, science]
+  stats:
+    hp: 10
+    speed: 4
+    max_melee_hit: 3
+    max_ranged_hit: 4
+    max_science_hit: 5
+`)
+	if issues := validateMobs(newSource(dir)); len(issues) != 0 {
+		t.Fatalf("valid combat mob must validate cleanly, got: %+v", issues)
+	}
+}
diff --git a/internal/validate/polish_destructive_fail_test.go b/internal/validate/polish_destructive_fail_test.go
new file mode 100644
index 0000000..8c9a038
--- /dev/null
+++ b/internal/validate/polish_destructive_fail_test.go
@@ -0,0 +1,64 @@
+package validate
+
+// Regression: the "a failable recipe must not yield nothing" invariant was
+// enforced only for smelting (and only by a shipped-data test), so a future
+// cooking/fletching/combine recipe with a success formula (or a declared
+// fail_message) and no fail product would silently consume materials and
+// return nothing. validateCraftBlocks now warns on it.
+
+import (
+	"path/filepath"
+	"testing"
+)
+
+func writePolishCraftRecipe(t *testing.T, dir, craftBody string) {
+	t.Helper()
+	writeFile(t, filepath.Join(dir, "items", "polish_in.yaml"), "id: polish_in\nname: polish in\n")
+	writeFile(t, filepath.Join(dir, "items", "polish_out.yaml"),
+		"id: polish_out\nname: polish out\ncraft:\n  - type: cooking\n"+craftBody+
+			"    ingredients:\n      - items: [polish_in]\n        quantity: 1\n")
+}
+
+func TestCraftFailableWithoutFailProductIsWarned(t *testing.T) {
+	cases := []struct {
+		name      string
+		craftBody string
+	}{
+		{"success formula", "    success:\n      base: 0.5\n      cap: 0.95\n      per_level: 0.02\n"},
+		{"declared fail_message", "    fail_message: You burn it.\n"},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			dir := t.TempDir()
+			writePolishCraftRecipe(t, dir, c.craftBody)
+			issues := validateCraftBlocks(newSource(dir))
+			if !containsMsg(issues, "failure destroys materials with no fail product") {
+				t.Fatalf("expected a destructive-failure warning, got: %+v", issues)
+			}
+			for _, issue := range issues {
+				if issue.Level != "WARN" {
+					t.Fatalf("destructive failure must be a WARN, got: %+v", issue)
+				}
+			}
+		})
+	}
+}
+
+func TestCraftWithFailProductOrGuaranteedSuccessDoesNotWarn(t *testing.T) {
+	cases := []struct {
+		name      string
+		craftBody string
+	}{
+		{"declares a fail product", "    fail: polish_in\n    success:\n      base: 0.5\n      cap: 0.95\n      per_level: 0.02\n"},
+		{"success cannot fall below 1", "    success:\n      base: 1.0\n      cap: 1.0\n      per_level: 0\n"},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			dir := t.TempDir()
+			writePolishCraftRecipe(t, dir, c.craftBody)
+			if issues := validateCraftBlocks(newSource(dir)); containsMsg(issues, "failure destroys materials with no fail product") {
+				t.Fatalf("did not expect a destructive-failure warning, got: %+v", issues)
+			}
+		})
+	}
+}