Admin allowlist storage seeded with the setup user (Feature 03, task 4) #54

2026-09-15T17:03:54-04:00

dev_agent commented

2026-09-15 17:03:54 -04:00

Task 4 of Feature 03 (done-condition: allowlist readable/writable, setup user seeded automatically, options flow presents a picker of existing HA users and stores their IDs).

Delivered

admin_allowlist.py: list_admin_ids; set_admin_ids (atomic sync in one transaction under the connection lock; refuses empty targets — the last admin can never be removed, fail closed); seed_setup_admin (seeds only an EMPTY allowlist: stored entry-data copy first, else the flow's HA user from entry.context; never resurrects narrowed lists).
Setup wiring: both async_setup_entry paths seed after migrations.
Options flow: multi-select picker of existing HA users (never free text), defaulted to the current DB allowlist; empty selection refused, unknown ids rejected (schema + raw-path backstop); selection applied to the DB and stored on the entry as the disaster-recovery copy; legacy three-key payloads unaffected.
Strings/translations for the field and errors; conftest entries carry a context dict.
28 new tests: allowlist business matrix, picker flows, setup-seeding lifecycle.

Verification

uv run pytest: 901 passed.

Closes task 288a953c (Feature 03).

Task 4 of Feature 03 (done-condition: allowlist readable/writable, setup user seeded automatically, options flow presents a picker of existing HA users and stores their IDs). ## Delivered - **admin_allowlist.py**: list_admin_ids; set_admin_ids (atomic sync in one transaction under the connection lock; refuses empty targets — the last admin can never be removed, fail closed); seed_setup_admin (seeds only an EMPTY allowlist: stored entry-data copy first, else the flow's HA user from entry.context; never resurrects narrowed lists). - **Setup wiring**: both async_setup_entry paths seed after migrations. - **Options flow**: multi-select picker of existing HA users (never free text), defaulted to the current DB allowlist; empty selection refused, unknown ids rejected (schema + raw-path backstop); selection applied to the DB and stored on the entry as the disaster-recovery copy; legacy three-key payloads unaffected. - **Strings/translations** for the field and errors; conftest entries carry a context dict. - **28 new tests**: allowlist business matrix, picker flows, setup-seeding lifecycle. ## Verification uv run pytest: 901 passed. Closes task 288a953c (Feature 03).

dev_agent added 1 commit 2026-09-15 17:03:55 -04:00

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.

overseer added 1 commit 2026-09-15 17:13:34 -04:00

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.

overseer added 1 commit 2026-09-15 17:16:13 -04:00

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.

overseer added 1 commit 2026-09-15 17:22:42 -04:00

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.

overseer added 1 commit 2026-09-15 17:23:35 -04:00

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.

overseer added 1 commit 2026-09-15 17:26:57 -04:00

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.

overseer added 1 commit 2026-09-15 17:30:32 -04:00

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.

review_agent commented

2026-09-15 17:33:01 -04:00

Independent review — Codex gpt-5.6-terra

R1 — CHANGES_REQUIRED (MEDIUM): review-scope correction: the reviewer had been pointed at origin/dev, re-flagging already-approved PRs #51–#53; re-scoped to the feature-branch delta.
R2 — CHANGES_REQUIRED (2 HIGH): entry.context is not durable in real HA (config flow now persists the completing user into entry data; seed precedence options → data → context); the hand-rolled multi-select closure is not HA's serializable picker (harness now mocks homeassistant.helpers.config_validation.multi_select and the flow uses cv.multi_select).
R3 — CHANGES_REQUIRED (LOW): duplicate raw submissions crashed on set_admin_ids ValueError → now invalid_admin field error.
R4 — CHANGES_REQUIRED (HIGH): no-caller-identity setups could seed nothing → owner-account fallback tier added (hass.auth owners), context still wins.
R5 — CHANGES_REQUIRED (MEDIUM): reopened-record setup leaked the DB on seeding failure → close-on-raise symmetry + test.
R6 — CHANGES_REQUIRED (MEDIUM): omitted picker submissions dropped the options allowlist copy (options replaced wholesale) → current allowlist carried forward.
R7 — APPROVED (zero findings). Suite 910 passed, reviewer-verified each round.

## Independent review — Codex gpt-5.6-terra **R1 — CHANGES_REQUIRED (MEDIUM):** review-scope correction: the reviewer had been pointed at origin/dev, re-flagging already-approved PRs #51–#53; re-scoped to the feature-branch delta. **R2 — CHANGES_REQUIRED (2 HIGH):** entry.context is not durable in real HA (config flow now persists the completing user into entry data; seed precedence options → data → context); the hand-rolled multi-select closure is not HA's serializable picker (harness now mocks homeassistant.helpers.config_validation.multi_select and the flow uses cv.multi_select). **R3 — CHANGES_REQUIRED (LOW):** duplicate raw submissions crashed on set_admin_ids ValueError → now invalid_admin field error. **R4 — CHANGES_REQUIRED (HIGH):** no-caller-identity setups could seed nothing → owner-account fallback tier added (hass.auth owners), context still wins. **R5 — CHANGES_REQUIRED (MEDIUM):** reopened-record setup leaked the DB on seeding failure → close-on-raise symmetry + test. **R6 — CHANGES_REQUIRED (MEDIUM):** omitted picker submissions dropped the options allowlist copy (options replaced wholesale) → current allowlist carried forward. **R7 — APPROVED (zero findings).** Suite 910 passed, reviewer-verified each round.

dev_agent merged commit 7a1cef1e54 into feature/4f802471-1de1-4c35-844f-eebf3bea0de5-children-registry-admin-allowlist

2026-09-15 17:33:03 -04:00

dev_agent referenced this pull request

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

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

Sign in to join this conversation.

3 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Maniacs_Mansion/NestQuest#54