From f0fa39660c7a337d8d775871abe4cd0d66e98b2c Mon Sep 17 00:00:00 2001 From: Joshua King Date: Tue, 15 Sep 2026 17:16:10 -0400 Subject: [PATCH] Review fix: duplicate admin selections surface as a field error 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. --- custom_components/nestquest/options_flow.py | 5 ++++ tests/test_options_flow.py | 29 +++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/custom_components/nestquest/options_flow.py b/custom_components/nestquest/options_flow.py index 2efe70a..9ab8214 100644 --- a/custom_components/nestquest/options_flow.py +++ b/custom_components/nestquest/options_flow.py @@ -184,6 +184,11 @@ class NestQuestOptionsFlow(config_entries.OptionsFlowWithConfigEntry): # backs the raw-input path up so an unknown id can never # reach the allowlist. errors[CONF_ADMIN_USER_IDS] = "invalid_admin" + elif admin_ids_present and len(set(admin_ids)) != len(admin_ids): + # set_admin_ids refuses duplicates with ValueError; a flow + # submission must surface that as a field error instead of + # an unhandled crash. + errors[CONF_ADMIN_USER_IDS] = "invalid_admin" if errors: return self.async_show_form( step_id="init", diff --git a/tests/test_options_flow.py b/tests/test_options_flow.py index b93b4f2..0830b5c 100644 --- a/tests/test_options_flow.py +++ b/tests/test_options_flow.py @@ -576,3 +576,32 @@ def test_options_flow_submit_without_admin_key_keeps_allowlist(tmp_path) -> None assert await_gather(list_admin_ids(database)) == ["u1"] finally: _run(database.close()) + + +def test_options_flow_duplicate_selection_refused(tmp_path) -> None: + """Duplicate ids surface as a field error, not an unhandled + ValueError from the business layer.""" + database = _open_allowlist_db(tmp_path, "picker-dupe.db") + try: + from custom_components.nestquest.admin_allowlist import ( + list_admin_ids, + set_admin_ids, + ) + + _run(set_admin_ids(database, ["u1"])) + entry = _make_entry() + flow = _make_flow( + entry, + users=[_user("u1", "Joshua"), _user("u2", "Sam")], + database=database, + ) + result = _run( + flow.async_step_init( + {**VALID_INPUT, CONF_ADMIN_USER_IDS: ["u2", "u2"]} + ) + ) + assert result["type"] == "form" + assert result["errors"] == {CONF_ADMIN_USER_IDS: "invalid_admin"} + assert await_gather(list_admin_ids(database)) == ["u1"] + finally: + _run(database.close())