Feature 03: Children Registry and Admin Allowlist -> dev #56

2026-09-15T17:38:23-04:00

dev_agent commented

2026-09-15 17:38:23 -04:00

Completes Feature 03 (identity layer), merged via gate-approved task PRs #51–#55:

#51: child-profile business layer (create/edit, required+trimmed names, duplicate-name warning).
#52: deactivation without data loss (rows intact, no-delete guard; review fixed non-bool is_active coercion).
#53: reorder for panel display order (complete-permutation contract, partial/unknown rejected atomically; review fixed bool child_id coercion).
#54: admin allowlist storage + setup seeding (fail-closed writes refusing empty allowlists, options-flow picker of existing HA users via cv.multi_select, persisted flow-user copy, owner-account fallback; 7 review rounds).
#55: fail-closed is_admin resolver (approved round 1).

Suite on the feature branch: 913 passed (85 new tests across the two business modules).

Completes Feature 03 (identity layer), merged via gate-approved task PRs #51–#55: - #51: child-profile business layer (create/edit, required+trimmed names, duplicate-name warning). - #52: deactivation without data loss (rows intact, no-delete guard; review fixed non-bool is_active coercion). - #53: reorder for panel display order (complete-permutation contract, partial/unknown rejected atomically; review fixed bool child_id coercion). - #54: admin allowlist storage + setup seeding (fail-closed writes refusing empty allowlists, options-flow picker of existing HA users via cv.multi_select, persisted flow-user copy, owner-account fallback; 7 review rounds). - #55: fail-closed is_admin resolver (approved round 1). Suite on the feature branch: 913 passed (85 new tests across the two business modules).

dev_agent added 23 commits 2026-09-15 17:38:23 -04:00

Child-profile business layer (Feature 03, task 1) 61b77cc2db

Business-layer functions over ChildrenDao for creating and editing
child profiles, so the Feature 09 service gate never touches the DAO
directly.

- children.py: create_child (display name required and trimmed,
  whitespace-only rejected; duplicate display name allowed but logged
  as a warning, case-insensitive and whitespace-normalised on both
  sides), edit_child (_UNSET sentinel for leave-alone; returns the
  updated record; renaming onto a different child's name warns, own
  name does not), set_child_active (the only removal path), and
  list_children (display order). Explicit None for optional fields
  raises instead of silently no-oping: the DAO's update deliberately
  writes only provided fields and cannot express NULL, so a clear
  request must not masquerade as one. UTC second-precision stamps per
  the timestamp policy.
- tests/test_children_registry.py: 27 tests covering the typed
  round-trips, name validation matrix, duplicate-name warning
  behaviour (create and rename, including the rename-to-self
  non-warning), partial edits, refusal of unsupported clears,
  deactivation round-trip and list ordering.

Review fix: duplicate-name scan excludes only the edited child 712527d5ba

Review round 1 (Codex gpt-5.6-terra) found: a case-only edit could
suppress the duplicate-name warning even when a DIFFERENT child
already carried the normalised name, because the edit path compared
only against the child's own old name. The scan now always runs on
the resulting state and excludes only the child being edited
(exclude_child_id), so case-only edits and stay-duplicated renames
warn like any other collision; renaming to your own name stays
warning-free when no other child holds the name. Tests: new
case-only-collision test, stay-duplicated rename assertion, and a
split self-rename test; suite at 848 passed.

Review fix: edit_child/set_child_active update+readback atomic 5578507ce8

Review round 2 (Codex gpt-5.6-terra) found both functions returned a
readback that a concurrent writer could overwrite between the UPDATE
and the SELECT, so the returned record could be the other caller's
values. Both now run existence-check, duplicate scan, UPDATE and
readback inside the DAOs' shared connection-scoped lock. Two gated
concurrency tests prove the contract deterministically: task A paused
at its UPDATE while task B queues, each returns its own values, final
stored state is the last writer's. Suite: 850 passed.

Review fix: race tests read via the DAO, not raw SQL probes 09363a57cf

The children/admin_users leak guard forbids children SQL outside the
DAO and its sanctioned test files; the two gated concurrency tests
probed the children table raw. They now assert via ChildrenDao.get.
Suite: 850 passed.

Review fix: serialize create_child's duplicate scan with the insert d5d972b872

Review round 3 (Codex gpt-5.6-terra) found two concurrent creates of
the same normalised name could both pass the duplicate scan before
either INSERT committed, inserting a duplicate with no warning. The
scan and INSERT now run inside the shared connection-scoped lock, so
the second create always sees the first's committed row and warns.
A gated concurrent-create test proves exactly one warning fires; a
sequential-duplicate warning assertion rode along. Suite: 851 passed.

Merge pull request 'Create and edit child profiles (Feature 03, task 1)' (#51 ) from task/90a3a7a3-child-profiles into feature/4f802471-1de1-4c35-844f-eebf3bea0de5-children-registry-admin-allowlist 6108755f0c

Deactivation preserves every referencing row (Feature 03, task 2) 1d247fbca1

Tests for the no-data-loss deactivation contract over the existing
set_child_active business function: a child with a definition,
instance and completion event keeps all three rows byte-identical
through deactivate and reactivate, is excluded from (and restored to)
the active list, and stays in the full list. A structural guard proves
the no-delete property: no module carries DELETE targeting children
and ChildrenDao exposes no delete-shaped method. Suite: 853 passed.

Fix leak-guard collision: reword no-delete guard docstring 5c74034f81

The guard test's docstring contained the phrase matching the
children/admin_users leak-guard pattern; reworded. Full suite green
before commit this time: 853 passed.

Review fix: set_child_active rejects non-bool is_active cfe1609df2

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.

Merge pull request 'Deactivate and reactivate a child without data loss (Feature 03, task 2)' (#52 ) from task/35e97242-child-deactivate into feature/4f802471-1de1-4c35-844f-eebf3bea0de5-children-registry-admin-allowlist ecffa5bb0e

Reorder children for panel display order (Feature 03, task 3) 7a585afb9e

- ChildrenDao.reorder now requires a COMPLETE permutation of the
  children table: partial lists (missing an existing child), unknown
  ids and duplicates are rejected with ValueError before any write,
  inside the existing transaction and connection lock (check and
  write atomic). Empty list is the maximally partial reorder against
  a non-empty table; against an empty table it stays a no-op. The two
  DAO tests that encoded the old permissive semantics (silent no-op
  on empty/unknown) are updated to the new contract.
- children.py gains reorder_children: the Feature 09-facing wrapper,
  which additionally rejects non-integer ids up front (bools and
  floats would silently compare against stored integer ids at the SQL
  layer).
- Business tests: round-trip (active list returns new order), partial/
  unknown/duplicate/non-int rejections with order-unchanged proofs,
  empty-list rejection, and inactive children keeping positions in a
  whole-table permutation. Suite: 866 passed.

Review fix: strict non-bool int child_id validation on all three calls 5fe6cb72ab

Review round 1 (Codex gpt-5.6-terra): SQLite binds bools as integers,
so edit_child(True, ...) would silently edit child 1 and
set_child_active had the same hole (reorder already validated its
list). A shared _validate_child_id guard now runs before lookup in
edit_child, set_child_active and reorder_children's list scan, with a
coercible-id test matrix for both call shapes (True/False/1.0/"1"/
None) asserting the refused call leaves the profile untouched. Suite:
875 passed.

Merge pull request 'Reorder children for panel display order (Feature 03, task 3)' (#53 ) from task/31e80e1a-child-reorder into feature/4f802471-1de1-4c35-844f-eebf3bea0de5-children-registry-admin-allowlist b7f7e092a0

Admin allowlist storage seeded with the setup user (Feature 03, task 4) 0d61ea2c88

- admin_allowlist.py: list_admin_ids / set_admin_ids (atomic sync of
  the allowlist inside one transaction under the connection lock;
  refuses empty targets so the last admin can never be removed and
  the allowlist can never be emptied — fail closed) and
  seed_setup_admin (seeds ONLY an empty allowlist: stored entry-data
  copy first — disaster recovery for a lost database — else the HA
  user who completed the config flow from the entry context;
  non-empty lists are never resurrected).
- __init__.py: both setup paths (first setup and the reopened-record
  path) seed after migrations; a failed seed closes the database and
  re-raises through the existing error handling.
- options_flow.py: the admin picker is a multi-select of EXISTING HA
  users (hass.auth.async_get_users), defaulted to the current
  database allowlist, membership-checked at the schema level and
  backed up on the raw-input path; empty selections are refused
  (no_admins), unknown ids rejected (invalid_admin); the selection is
  applied to the DB when the runtime record is reachable and always
  stored on the entry as the recovery copy. Submissions without the
  key (legacy raw payloads) keep the three-key behaviour untouched.
- const.CONF_ADMIN_USER_IDS; strings + translations for the field and
  the two errors; conftest entries gain a context dict (HA's entry
  context carries the flow user).
- Tests: 20 allowlist business tests (round-trips, refusal matrix,
  seed precedence and narrowed-list persistence), 5 picker flow tests,
  3 setup-seeding lifecycle tests. Suite: 901 passed.

Review fixes: persisted flow-user admin seed and real cv.multi_select d90c170460

Review rounds 1-2 (Codex gpt-5.6-terra) found two HIGHs against real HA:
- entry.context is not a durable record — the config flow now persists
  the completing HA user into entry data (CONF_ADMIN_USER_IDS) and the
  seed precedence becomes: options copy (the current list) first, then
  the config flow's entry-data copy, then the context user as a last
  resort. Config-flow tests cover the persisted user and the
  context-free path; a seed-precedence test covers options > data.
- the hand-rolled closure could not serialize as an HA multi-select.
  The harness now mocks homeassistant.helpers.config_validation with a
  faithful multi_select, and the flow uses cv.multi_select exactly as
  real HA code does. Suite: 904 passed.

Review fix: duplicate admin selections surface as a field error f0fa39660c

Review round 3 (Codex gpt-5.6-terra, LOW): a duplicate raw submission
reached set_admin_ids and crashed the flow with an unhandled
ValueError; the flow now rejects duplicates as invalid_admin with a
test proving the database is untouched. Suite: 905 passed.

Review fix: owner-account fallback seed for first setup 068ace5693

Review round 4 (Codex gpt-5.6-terra, HIGH): relying on the flow
context alone can leave first setup ownerless. seed_setup_admin gains
an owner_ids tier: when no persisted copy and no context user exist,
every HA owner account is seeded (owners can already do everything in
HA — no privilege escalation). __init__ resolves owners via
hass.auth.async_get_users on both setup paths; conftest's hass gains
the auth surface. Tests: owner fallback seeds only owners, context
user still wins over owners, owner_ids argument validation. Suite:
910 passed.

Test fix: malformed owner_ids validated before the seed's early return 7ebd893823

The seed validates candidates only when the allowlist is empty; the
test's successful first seed masked the malformed case. The rejection
now runs first. Suite: 908 passed.

Review fix: reopened-record setup closes the database when seeding fails 0e4f890ef9

Review round 5 (Codex gpt-5.6-terra, MEDIUM): the reopened-runtime
setup path leaked the just-opened connection if owner lookup or
seed_setup_admin raised, so HA's setup retries could accumulate open
handles. Seeding on that path is now wrapped symmetrically with the
first-setup path: any failure closes the database before re-raising,
with a test that fails seeding on a re-setup and asserts the close.
Suite: 909 passed.

Review fix: omitted picker submissions carry the allowlist forward 913d3658b8

Review round 6 (Codex gpt-5.6-terra, MEDIUM): HA replaces entry.options
wholesale, so a submission omitting the picker dropped the allowlist
copy and left only the stale config-flow copy in entry data — a later
database loss would resurrect removed admins. Omitted submissions now
carry the current DB allowlist forward on the options copy. The
legacy-path test updated to assert the carried copy. Suite: 910 passed.

Merge pull request 'Admin allowlist storage seeded with the setup user (Feature 03, task 4)' (#54 ) from task/288a953c-admin-allowlist into feature/4f802471-1de1-4c35-844f-eebf3bea0de5-children-registry-admin-allowlist 7a1cef1e54

Fail-closed is_admin resolver (Feature 03, task 5) 7cde256e2a

admin_allowlist.is_admin: True only for an allowlisted HA user id;
False for non-allowlisted, unknown, empty-string, untrimmed variants,
None, non-strings (bools, lists), and an empty allowlist (nobody is
admin rather than everybody). Never raises for malformed input —
malformed means not admin; DB errors propagate for the Feature 09
gate to handle. The last-admin removal refusal (set_admin_ids) is
re-asserted as keeping the verdict sound. Tests: full verdict matrix,
empty-allowlist fail-closed, refusal persistence. Suite: 912 passed.

Merge pull request 'Implement the fail-closed is_admin resolver (Feature 03, task 5)' (#55 ) from task/3f2eb268-is-admin-resolver into feature/4f802471-1de1-4c35-844f-eebf3bea0de5-children-registry-admin-allowlist c2cdbceb09

orchestrator-agent merged commit b3957ecedf into dev

2026-09-15 17:38:45 -04:00

dev_agent referenced this pull request

2026-09-15 19:05:39 -04:00

Release: Features 14, 03, 05 — quest model, children registry, presence engine (2026-09-15) #65

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Maniacs_Mansion/NestQuest#56