fix: validate limits policy scalar types

CHANGELOG.md

@@ -322,6 +322,10 @@ whose seams had diverged enough that several ports needed a different fix, and t
 
 ### Fixed
 
+- **Reject mismatched TOML scalar types for every `limits.*` policy field (#278).**
+  Quoted booleans and other coercible values now raise `PolicyError` instead of silently changing
+  the configured limit or enabling a disabled behavior.
+
 - **Contributor docs now name every obligation CI enforces.** `CONTRIBUTING.md` never mentioned
   `pyright` or the CHANGELOG, so its verify step — "`trunk check` and `uv run pytest -q` both
   pass" — sent contributors into a CI failure on the dedicated typecheck job. It now carries

src/bmad_loop/policy.py

@@ -672,6 +672,34 @@ def _validate_plugin_settings(name: str, raw: dict[str, Any], specs: Any) -> Non
             )
 
 
+def _limit_int(raw: dict[str, Any], key: str, default: int) -> int:
+    value = raw.get(key, default)
+    if isinstance(value, bool) or not isinstance(value, int):
+        raise PolicyError(f"limits.{key} must be an integer: got {value!r}")
+    return value
+
+
+def _limit_float(raw: dict[str, Any], key: str, default: float) -> float:
+    value = raw.get(key, default)
+    if isinstance(value, bool) or not isinstance(value, (int, float)):
+        raise PolicyError(f"limits.{key} must be a number: got {value!r}")
+    return float(value)
+
+
+def _limit_bool(raw: dict[str, Any], key: str, default: bool) -> bool:
+    value = raw.get(key, default)
+    if not isinstance(value, bool):
+        raise PolicyError(f"limits.{key} must be a boolean: got {value!r}")
+    return value
+
+
+def _limit_str(raw: dict[str, Any], key: str, default: str) -> str:
+    value = raw.get(key, default)
+    if not isinstance(value, str):
+        raise PolicyError(f"limits.{key} must be a string: got {value!r}")
+    return value
+
+
 def load(path: Path | None) -> Policy:
     """Load policy from a TOML file; a missing file yields all defaults.
 
@@ -736,63 +764,46 @@ def loads(text: str, plugin_schemas: dict[str, Any] | None = None) -> Policy:
             f"gates.retrospective must be one of {sorted(RETRO_MODES)}: got {gates.retrospective!r}"
         )
 
-    # the budget knobs gate enforce-mode termination, so a coerced bool/float
-    # (true -> 1 token) must be rejected, not silently accepted (same rule as
-    # scm.preserve_keep below). The per-story cap is checked here on the same
-    # terms even though it only warns: a coerced `true` caps a whole story at 1
-    # token, which now fires at the first session boundary of every story.
-    max_tokens_per_story = limits_d.get("max_tokens_per_story", LimitsPolicy.max_tokens_per_story)
-    if isinstance(max_tokens_per_story, bool) or not isinstance(max_tokens_per_story, int):
-        raise PolicyError(
-            f"limits.max_tokens_per_story must be an integer: got {max_tokens_per_story!r}"
-        )
-    max_tokens_per_session = limits_d.get(
-        "max_tokens_per_session", LimitsPolicy.max_tokens_per_session
-    )
-    if isinstance(max_tokens_per_session, bool) or not isinstance(max_tokens_per_session, int):
-        raise PolicyError(
-            f"limits.max_tokens_per_session must be an integer: got {max_tokens_per_session!r}"
-        )
-    session_budget_grace_s = limits_d.get(
-        "session_budget_grace_s", LimitsPolicy.session_budget_grace_s
-    )
-    if isinstance(session_budget_grace_s, bool) or not isinstance(session_budget_grace_s, int):
-        raise PolicyError(
-            f"limits.session_budget_grace_s must be an integer: got {session_budget_grace_s!r}"
-        )
-
     limits = LimitsPolicy(
-        max_review_cycles=int(limits_d.get("max_review_cycles", LimitsPolicy.max_review_cycles)),
-        max_dev_attempts=int(limits_d.get("max_dev_attempts", LimitsPolicy.max_dev_attempts)),
-        max_followup_reviews=int(
-            limits_d.get("max_followup_reviews", LimitsPolicy.max_followup_reviews)
+        max_review_cycles=_limit_int(limits_d, "max_review_cycles", LimitsPolicy.max_review_cycles),
+        max_dev_attempts=_limit_int(limits_d, "max_dev_attempts", LimitsPolicy.max_dev_attempts),
+        max_followup_reviews=_limit_int(
+            limits_d, "max_followup_reviews", LimitsPolicy.max_followup_reviews
+        ),
+        session_timeout_min=_limit_int(
+            limits_d, "session_timeout_min", LimitsPolicy.session_timeout_min
+        ),
+        git_timeout_s=_limit_int(limits_d, "git_timeout_s", LimitsPolicy.git_timeout_s),
+        teardown_grace_s=_limit_int(limits_d, "teardown_grace_s", LimitsPolicy.teardown_grace_s),
+        stop_without_result_nudges=_limit_int(
+            limits_d, "stop_without_result_nudges", LimitsPolicy.stop_without_result_nudges
+        ),
+        dev_stall_grace_s=_limit_int(limits_d, "dev_stall_grace_s", LimitsPolicy.dev_stall_grace_s),
+        dev_stall_nudges=_limit_int(limits_d, "dev_stall_nudges", LimitsPolicy.dev_stall_nudges),
+        dev_stall_nudges_cap=_limit_int(
+            limits_d, "dev_stall_nudges_cap", LimitsPolicy.dev_stall_nudges_cap
+        ),
+        workflow_stall_nudges_cap=_limit_int(
+            limits_d, "workflow_stall_nudges_cap", LimitsPolicy.workflow_stall_nudges_cap
         ),
-        session_timeout_min=int(
-            limits_d.get("session_timeout_min", LimitsPolicy.session_timeout_min)
+        dev_contract_nudge=_limit_bool(
+            limits_d, "dev_contract_nudge", LimitsPolicy.dev_contract_nudge
         ),
-        git_timeout_s=int(limits_d.get("git_timeout_s", LimitsPolicy.git_timeout_s)),
-        teardown_grace_s=int(limits_d.get("teardown_grace_s", LimitsPolicy.teardown_grace_s)),
-        stop_without_result_nudges=int(
-            limits_d.get("stop_without_result_nudges", LimitsPolicy.stop_without_result_nudges)
+        max_tokens_per_story=_limit_int(
+            limits_d, "max_tokens_per_story", LimitsPolicy.max_tokens_per_story
         ),
-        dev_stall_grace_s=int(limits_d.get("dev_stall_grace_s", LimitsPolicy.dev_stall_grace_s)),
-        dev_stall_nudges=int(limits_d.get("dev_stall_nudges", LimitsPolicy.dev_stall_nudges)),
-        dev_stall_nudges_cap=int(
-            limits_d.get("dev_stall_nudges_cap", LimitsPolicy.dev_stall_nudges_cap)
+        cache_read_weight=_limit_float(
+            limits_d, "cache_read_weight", LimitsPolicy.cache_read_weight
         ),
-        workflow_stall_nudges_cap=int(
-            limits_d.get("workflow_stall_nudges_cap", LimitsPolicy.workflow_stall_nudges_cap)
+        session_budget_mode=_limit_str(
+            limits_d, "session_budget_mode", LimitsPolicy.session_budget_mode
         ),
-        dev_contract_nudge=bool(
-            limits_d.get("dev_contract_nudge", LimitsPolicy.dev_contract_nudge)
+        max_tokens_per_session=_limit_int(
+            limits_d, "max_tokens_per_session", LimitsPolicy.max_tokens_per_session
         ),
-        max_tokens_per_story=max_tokens_per_story,
-        cache_read_weight=float(limits_d.get("cache_read_weight", LimitsPolicy.cache_read_weight)),
-        session_budget_mode=str(
-            limits_d.get("session_budget_mode", LimitsPolicy.session_budget_mode)
+        session_budget_grace_s=_limit_int(
+            limits_d, "session_budget_grace_s", LimitsPolicy.session_budget_grace_s
         ),
-        max_tokens_per_session=max_tokens_per_session,
-        session_budget_grace_s=session_budget_grace_s,
     )
     if limits.max_review_cycles < 1 or limits.max_dev_attempts < 1:
         raise PolicyError("limits.max_review_cycles and limits.max_dev_attempts must be >= 1")

tests/test_policy.py

@@ -434,6 +434,41 @@ def test_dev_contract_nudge_default_parse_and_template():
     assert doc["limits"]["dev_contract_nudge"] == policy.LimitsPolicy.dev_contract_nudge
 
 
+def test_dev_contract_nudge_rejects_non_boolean():
+    with pytest.raises(policy.PolicyError, match=r"limits\.dev_contract_nudge must be a boolean"):
+        policy.loads('[limits]\ndev_contract_nudge = "false"\n')
+
+
+@pytest.mark.parametrize(
+    "key",
+    [
+        "max_review_cycles",
+        "max_dev_attempts",
+        "max_followup_reviews",
+        "session_timeout_min",
+        "git_timeout_s",
+        "teardown_grace_s",
+        "stop_without_result_nudges",
+        "dev_stall_grace_s",
+        "dev_stall_nudges",
+        "dev_stall_nudges_cap",
+        "workflow_stall_nudges_cap",
+        "max_tokens_per_story",
+        "max_tokens_per_session",
+        "session_budget_grace_s",
+    ],
+)
+def test_limits_integer_fields_reject_non_integer(key):
+    with pytest.raises(policy.PolicyError, match=rf"limits\.{key} must be an integer"):
+        policy.loads(f'[limits]\n{key} = "1"\n')
+
+
+@pytest.mark.parametrize("bad", ["true", '"0.5"'])
+def test_cache_read_weight_rejects_non_number(bad):
+    with pytest.raises(policy.PolicyError, match=r"limits\.cache_read_weight must be a number"):
+        policy.loads(f"[limits]\ncache_read_weight = {bad}\n")
+
+
 def test_session_budget_mode_default_parse_and_template():
     import tomllib
 
@@ -451,6 +486,11 @@ def test_invalid_session_budget_mode():
         policy.loads('[limits]\nsession_budget_mode = "sometimes"\n')
 
 
+def test_session_budget_mode_rejects_non_string():
+    with pytest.raises(policy.PolicyError, match=r"limits\.session_budget_mode must be a string"):
+        policy.loads("[limits]\nsession_budget_mode = 1\n")
+
+
 def test_max_tokens_per_story_default_and_parse():
     assert policy.loads("").limits.max_tokens_per_story == 2_000_000
     assert policy.loads("[limits]\nmax_tokens_per_story = 500\n").limits.max_tokens_per_story == 500