From cfe1609df25ff7e0add1691bb3ce2c396228f3da Mon Sep 17 00:00:00 2001
From: Joshua King <joshuaking.mba@gmail.com>
Date: Tue, 15 Sep 2026 16:35:18 -0400
Subject: [PATCH] Review fix: set_child_active rejects non-bool is_active

Review round 1 (Codex gpt-5.6-terra): the DAO's int() silently coerced
strings and numerics (1, 0, 1.0) into an unintended active state.
The business layer now requires a real bool, before the existence
check's caller-intent can be lost, with a coercible-input test matrix;
the refused transition leaves the record untouched. Suite: 860 passed.
---
 custom_components/nestquest/children.py |  9 ++++++++-
 tests/test_children_registry.py         | 15 +++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/custom_components/nestquest/children.py b/custom_components/nestquest/children.py
index 4e1d56e..315fdf8 100644
--- a/custom_components/nestquest/children.py
+++ b/custom_components/nestquest/children.py
@@ -213,7 +213,10 @@ async def set_child_active(
 
     Deactivation hides the child from the panel without deleting their
     history (feature guardrail); this is the only removal path.
-    Raises ValueError when the child does not exist.
+    ``is_active`` must be a real bool: the DAO's ``int()`` would
+    otherwise silently coerce strings and numerics ("1", 0) into a
+    state the caller never asked for.  Raises ValueError when the
+    child does not exist.
 
     Atomicity: existence check, UPDATE and readback run inside the
     connection-scoped lock, so a concurrent opposite transition cannot
@@ -223,6 +226,10 @@ async def set_child_active(
         dao = ChildrenDao(database)
         if await dao.get(child_id) is None:
             raise ValueError(f"child {child_id} does not exist")
+        if not isinstance(is_active, bool):
+            raise ValueError(
+                f"is_active must be a real bool, got {is_active!r}"
+            )
         await dao.set_active(child_id, is_active)
         updated = await dao.get(child_id)
     assert updated is not None
diff --git a/tests/test_children_registry.py b/tests/test_children_registry.py
index 3f977fb..e3d28f6 100644
--- a/tests/test_children_registry.py
+++ b/tests/test_children_registry.py
@@ -441,6 +441,21 @@ def test_set_child_active_missing_raises(tmp_path) -> None:
     _with_db(tmp_path, "set-active-missing.db")(_body)
 
 
+@pytest.mark.parametrize("bad", ["1", "0", 1, 0, 1.0, None, "true"])
+def test_set_child_active_rejects_non_bool(tmp_path, bad) -> None:
+    """The DAO's int() would silently coerce "1"/0/1.0 into a state the
+    caller never asked for; the business layer refuses non-bools."""
+    async def _body(database):
+        child = await create_child(database, "Ada")
+        with pytest.raises(ValueError, match="is_active must be a real bool"):
+            await set_child_active(database, child.id, bad)
+        # The refused transition left the child untouched.
+        assert (await list_children(database))[0].is_active is True
+        return None
+
+    _with_db(tmp_path, "set-active-non-bool.db")(_body)
+
+
 # ---------------------------------------------------------------------------
 # Concurrency: edit/set_active are atomic under the connection lock
 # ---------------------------------------------------------------------------